diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c29b20431..723e3b5f7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,57 +1,169 @@ -# Winit Contributing Guidelines +# Contribution Guidelines -## Scope -[See `FEATURES.md`](./FEATURES.md). When requesting or implementing a new Winit feature, you should -consider whether or not it's directly related to window creation or input handling. If it isn't, it -may be worth creating a separate crate that extends Winit's API to add that functionality. +This document contains guidelines for contributing code to winit. It has to be +followed in order for your patch to be approved and applied. +## Contributing -## Reporting an issue +Anyone can contribute to winit, however given that it's a cross platform +windowing toolkit getting certain changes incorporated upstream could be +challenging. -When reporting an issue, in order to help the maintainers understand what the problem is, please make -your description of the issue as detailed as possible: +To save your time it's wise to check already opened [pull requests][prs] and +[issues][issues]. In general, bug fixes and missing implementations are always +accepted, however new API proposals should go into the issue first. When in +doubt contact us on [matrix][matrix] or via opening an issue. -- if it is a bug, please provide a clear explanation of what happens, what should happen, and how to - reproduce the issue, ideally by providing a minimal program exhibiting the problem -- if it is a feature request, please provide a clear argumentation about why you believe this feature - should be supported by winit +### Submitting your work and handling review -## Making a pull request +All patches have to be sent on Github as [pull requests][prs]. To simplify your +life during review it's recommended to check the "give contributors write access +to the branch" checkbox. -When making a code contribution to winit, before opening your pull request, please make sure that: +#### Creating commits -- your patch builds with Winit's minimal supported rust version - Rust 1.70. -- you tested your modifications on all the platforms impacted, or if not possible, detail which platforms - were not tested, and what should be tested, so that a maintainer or another contributor can test them -- you updated any relevant documentation in winit -- you left comments in your code explaining any part that is not straightforward, so that the - maintainers and future contributors don't have to try to guess what your code is supposed to do -- your PR adds an entry to the changelog file if the introduced change is relevant to winit users. +When creating a commit, follow these general rules: + - Try to limit the first line (title) of the commit message to 50 characters, + but not more than 60 characters. + - The commit starts with an uppercase latter and everything else must be + lowercase, unless you quote a symbol, like `Window`. + - Use the body of the commit message to actually explain what your patch does + and why it is useful. Even if your patch is a one line fix, the description + is not limited in length and may span over multiple paragraphs. Use proper + English syntax, grammar and punctuation. The body is generally wrapped + at 72 characters. + - If you're a downstream user and your fixing your issue, consider to link it + like `Links: URL` in the end of the commit message. + - Try to address only one issue/topic per commit. + - Prefer describing your changes in imperative mood, e.g. *"make xyzzy do + frotz"* instead of *"[This patch] makes xyzzy do frotz"* or *"[I] changed + xyzzy to do frotz"*, as if you are giving orders to the codebase to change + its behaviour. + - When fixing an issue from https://github.com/rust-windowing/winit/issues, + ensure to have it include in the end of the commit. See other commits + on how it's done. + - When in doubt, follow the format and layout of the recent existing commits. - You needn't worry about the added entry causing conflicts, the maintainer that merges the PR will - handle those for you when merging (see below). -- if your PR affects the platform compatibility of one or more features or adds another feature, the - relevant sections in [`FEATURES.md`](https://github.com/rust-windowing/winit/blob/master/FEATURES.md#features) - should be updated. +The maintainers will fixup your commit message when applying the patch, but it's +still recommended to follow the rules above to make the life of maintainers +easier. -Once your PR is open, you can ask for a review by a maintainer of your platform. Winit's merging policy -is that a PR must be approved by at least two maintainers of winit before being merged, including -at least a maintainer of the platform (a maintainer making a PR themselves counts as approving it). +If you want to communicate something to reviewers, but it doesn't belong to +the commit message, write that under `--` on PR body. Your PR could look like: -Once your PR is deemed ready, the merging maintainer will take care of resolving conflicts in -`CHANGELOG.md` (but you must resolve other conflicts yourself). Doing this requires that you check the -"give contributors write access to the branch" checkbox when creating the PR. + ``` + TITLE (matches commit title) + + Body (matches commit body) + + -- + + Your arbitrary message goes here. + + Various checkboxes PR template required you to fill. + ``` + +#### Handling review + +During the review process certain events could require an action from your side, +to communication more efficient the common patterns and reactions are described +below. + +_Event:_ The CI fails to build, but it looks like not your fault. Not +communicating so, could result into maintainers not looking into your patch, +unless they CI that CI pass.\ +_Desired behavior:_ Write a message saying roughly the following "The CI failure +is unrelated", so maintainers will fix it for you. + +_Event:_ Collaborator requested review on your PR.\ +_Desired behavior:_ Once you address the request, you _should_ re-request review +with the github's UI. If you don't agree with what maintainer suggested, you +should object that and re-request the review. That will indicate that the +_ball_ is on maintainer's side. + +_Event:_ You've opened a PR, but maintainer shortly after commented that they +want to work on that _themselves_.\ +_Desired behavior:_ Discusses with maintainer their plans if they were not +outlined in the initial response, because such response means that they +are not interested in reviewing your code. Such thing could happen when +underestimating complexity of the task you're solving or when your patch +mandate certain downstream designs. + +[prs]: https://github.com/rust-windowing/winit/pulls +[issues]: https://github.com/rust-windowing/winit/issues +[matrix]: https://matrix.to/#/#rust-windowing:matrix.org ## Maintainers -The current maintainers for each platform are listed in the [CODEOWNERS](.github/CODEOWNERS) file. +Winit has plenty of maintainers with different backgrounds, experience level, +and reasons to be winit maintainer in the first place. To ensure that winit's +code quality is not decreasing over time and to make it easier to teach new +maintainers the _winit way of doing things_ the common policies and routines +are defined in this section. -## Release process +The current maintainers for each platform are listed in [this file][CODEOWNERS]. + +### Contributions handling + +The maintainer must ensure that the external contributions meet the winit's +quality standards. If it's not, it **is maintainer's responsibility** to bring +it on par, which includes: + + - Decline the patch if it tries to achieve short term goals instead of solving + the problem in general. In such case maintainer must communicate that **as + early as possible**, so contributors don't spend their time on something + that won't be accepted in the end. + - Ensure that formatting is consistent and `CHANGELOG` messages are clear + for the end users. + - Improve the commit message, so it'll be easier for other maintainers to + understand the motivation without going through all the discussions on the + particular patch/PR. + - Ensure that the proposed patch doesn't break platform parity. If the + breakage is desired by contributor, an issue should be opened to discuss + with other maintainers before merging. + - Always fix CI issues before merging if they don't originate from the + submitted work. + +However, maintainer must always give a leeway for external contributors, so they +don't feel discouraged contributing, for example: + + - Suggest a patch to resolve style issues, if it's the only issue with the + submitted work. Keep in mind that pushing the resolution yourself is not + desired, because contributor might not agree with what you did. + - Be more explicit on how things should be done if you doesn't like the + approach. + - Suggest to finish PR for them if they're absent for a while and you need the + proposed changes to move forward with something. In such case maintainer + must preserve attribution with `Co-authored-by`, `Suggested-by`, or keep + the original commiter. + - Rebase their work for them when massive changes to winit codebase were + introduced. + +When reviewing code of other maintainers all of the above is on the maintainer +who submitted the patch. Interested maintainers could help push the work over +the finish line, but teaching other maintainers should be preferred. + +When contributor is _regular_ in winit, the maintainer should slowly start +requiring contributor to match *maintainer* quality standards when writing +patches and commit messages. + +### Contributing + +When submitting a patch maintainer should follow the general contributing +guidelines, however all soft rules (e.g `Try to`), become strict. + +To make life simpler for other maintainers it's suggested to create your branch +under the project repository instead of your own fork. The naming scheme is +`github_user_name/branch_name`. Doing so will make your work easier to rebase +for other maintainers when you're absent. + +### Release process Given that winit is a widely used library, we should be able to make a patch releases at any time we want without blocking the development of new features. -To achieve these goals, a new branch is created for every new release. Releases and later patch releases are committed and tagged in this branch. +To achieve these goals, a new branch is created for every new release. Releases +and later patch releases are committed and tagged in this branch. The exact steps for an exemplary `0.2.0` release might look like this: 1. Initially, the version on the latest master is `0.1.0` @@ -67,3 +179,5 @@ When doing a patch release, the process is similar: 2. Checkout the `v0.2.x` branch 3. Cherry-pick the required non-breaking changes into the `v0.2.x` 4. Follow steps 3-7 of the regular release example + +[CODEOWNERS]: .github/CODEOWNERS