Skip to content

Commit 0ca9c61

Browse files
committed
Update "Classifying PRs" section to talk about D-Complex (#7216)
The current section does not talk about `D-Complex` and lists things like "adds unsafe code" as a reason to mark a PR `S-Controversial`. This is not how `D-Complex` and `S-Controversial` are being used at the moment. This PR lists what classifies a PR as `D-Complex` and what classifies a PR as `S-Controversial`. It also links to some PRs with each combination of labels to help give an idea for what this means in practice. cc #7211 which is doing a similar thing
1 parent 63a291c commit 0ca9c61

File tree

1 file changed

+50
-21
lines changed

1 file changed

+50
-21
lines changed

CONTRIBUTING.md

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -76,37 +76,66 @@ Check out our dedicated [Bevy Organization document](/docs/the_bevy_organization
7676

7777
### Classifying PRs
7878

79-
Our merge strategy relies on the classification of PRs into three categories: **trivial**, **uncontroversial** and **controversial**.
80-
When making PRs, try to split out more controversial changes from less controversial ones, in order to make your work easier to review and merge.
81-
PRs that are deemed controversial will receive the [`S-Controversial`](https://github.com/bevyengine/bevy/pulls?q=is%3Aopen+is%3Apr+label%3AS-Controversial) label, and will have to go through the more thorough review process.
79+
Our merge strategy relies on the classification of PRs on two axes:
8280

83-
PRs are trivial if there is no reasonable argument against them. This might include:
81+
* How controversial are the design decisions
82+
* How complex is the implementation
8483

85-
* Fixing dead links.
86-
* Removing dead code or dependencies.
87-
* Typo and grammar fixes.
84+
PRs with non-trivial design decisions are given the [`S-Controversial`] label. This indicates that
85+
the PR needs more thorough design review or an [RFC](https://github.com/bevyengine/rfcs), if complex enough.
8886

89-
PRs are controversial if there is serious design discussion required, or a large impact to contributors or users. Factors that increase controversy include:
87+
PRs that are non-trivial to review are given the [`D-Complex`] label. This indicates that the PR
88+
should be reviewed more thoroughly and by people with experience in the area that the PR touches.
89+
90+
When making PRs, try to split out more controversial changes from less controversial ones, in order to make your work easier to review and merge.
91+
It is also a good idea to try and split out simple changes from more complex changes if it is not helpful for then to be reviewed together.
9092

91-
1. Changes to project-wide workflow or style.
93+
Some things that are reason to apply the [`S-Controversial`] label to a PR:
94+
95+
1. Changes to a project-wide workflow or style.
9296
2. New architecture for a large feature.
93-
3. PRs where a serious tradeoff must be made.
97+
3. Serious tradeoffs were made.
9498
4. Heavy user impact.
9599
5. New ways for users to make mistakes (footguns).
96-
6. Introductions of `unsafe` code.
97-
7. Large-scale code reorganization.
98-
8. High levels of technical complexity.
99-
9. Adding a dependency.
100-
10. Touching licensing information (due to the level of precision required).
101-
11. Adding root-level files (due to the high level of visibility).
100+
6. Adding a dependency
101+
7. Touching licensing information (due to level of precision required).
102+
8. Adding root-level files (due to the high level of visibility)
103+
104+
Some things that are reason to apply the [`D-Complex`] label to a PR:
105+
106+
1. Introduction or modification of soundness relevent code (for example `unsafe` code)
107+
2. High levels of technical complexity.
108+
3. Large-scale code reorganization
109+
110+
Examples of PRs that are not [`S-Controversial`] or [`D-Complex`]:
111+
112+
* Fixing dead links.
113+
* Removing dead code or unused dependencies.
114+
* Typo and grammar fixes.
115+
* [Add `Mut::reborrow`](https://github.com/bevyengine/bevy/pull/7114)
116+
* [Add `Res::clone`](https://github.com/bevyengine/bevy/pull/4109)
117+
118+
Examples of PRs that are [`S-Controversial`] but not [`D-Complex`]:
119+
120+
* [Implement and require `#[derive(Component)]` on all component structs](https://github.com/bevyengine/bevy/pull/2254)
121+
* [Use default serde impls for Entity](https://github.com/bevyengine/bevy/pull/6194)
122+
123+
Examples of PRs that are not [`S-Controversial`] but are [`D-Complex`]:
124+
125+
* [Ensure `Ptr`/`PtrMut`/`OwningPtr` are aligned in debug builds](https://github.com/bevyengine/bevy/pull/7117)
126+
* [Replace `BlobVec`'s `swap_scratch` with a `swap_nonoverlapping`](https://github.com/bevyengine/bevy/pull/4853)
127+
128+
Examples of PRs that are both [`S-Controversial`] and [`D-Complex`]:
129+
130+
* [bevy_reflect: Binary formats](https://github.com/bevyengine/bevy/pull/6140)
102131

103-
Finally, changes are "relatively uncontroversial" if they are neither trivial or controversial.
104-
Most PRs should fall into this category.
132+
Some useful pull request queries:
105133

106-
Here are some useful pull request queries:
134+
* [PRs which need reviews and are not `D-Complex`](https://github.com/bevyengine/bevy/pulls?q=is%3Apr+-label%3AD-Complex+-label%3AS-Ready-For-Final-Review+-label%3AS-Blocked++)
135+
* [`D-Complex` PRs which need reviews](https://github.com/bevyengine/bevy/pulls?q=is%3Apr+label%3AD-Complex+-label%3AS-Ready-For-Final-Review+-label%3AS-Blocked)
107136

108-
* [Uncontroversial pull requests which have been reviewed and are ready for maintainers to merge](https://github.com/bevyengine/bevy/pulls?q=is%3Aopen+is%3Apr+label%3AS-Ready-For-Final-Review+-label%3AS-Controversial+-label%3AS-Blocked+-label%3AS-Adopt-Me+)
109-
* [Controversial pull requests which have been reviewed and are ready for final input from a Project Lead or SME](https://github.com/bevyengine/bevy/pulls?q=is%3Aopen+is%3Apr+label%3AS-Ready-For-Final-Review+label%3AS-Controversial+)
137+
[`S-Controversial`]: https://github.com/bevyengine/bevy/pulls?q=is%3Aopen+is%3Apr+label%3AS-Controversial
138+
[`D-Complex`]: https://github.com/bevyengine/bevy/pulls?q=is%3Aopen+is%3Apr+label%3AD-Complex
110139

111140
### Preparing Releases
112141

0 commit comments

Comments
 (0)