Skip to content

Commit

Permalink
docs(contrib): Suggest atomic commits with separate test commits
Browse files Browse the repository at this point in the history
This came up in a discussion with @torhovland.
I'm unsure what the right weight is to put behind these suggestions
- I don't want a lack of git history editing experience to get in the
  way of someone contributing
- If people do anything, I'd like to see the tests split out
  • Loading branch information
epage committed Jun 5, 2024
1 parent 34a6a87 commit e062175
Showing 1 changed file with 15 additions and 0 deletions.
15 changes: 15 additions & 0 deletions src/doc/contrib/src/process/working-on-cargo.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,21 @@ typically finish in under 30 minutes.
The reviewer might point out changes deemed necessary. Large or tricky changes
may require several passes of review and changes.

> **tip:** Prefer atomic commits where each commit is a single, complete, and coherent unit of work.
> For example, if your feature work leads to renaming a module, make the rename its own commit.
> However, adding an internal function that is unused is not complete of coherent.
>
> As part of your atomic commits, prefer adding tests as their own commit *before* any functionality changes.
> The tests should pass, showing the existing behavior.
> As functionality changes, the tests should be updated to pass.
> This makes it easier for reviewers and community members to understand the
> precise details of the side effects of your change and gives you confidence
> that your tests are verifying the right behavior.
>
> Examples:
> - [#13910: fix: remove symlink dir on Windows](https://github.com/rust-lang/cargo/pull/13910)
> - [#14006: fix(add): Avoid escaping double-quotes by using string literals](https://github.com/rust-lang/cargo/pull/14006)
### Status labeling

PRs will get marked with [labels] like [`S-waiting-on-review`] or [`S-waiting-on-author`] to indicate their status.
Expand Down

0 comments on commit e062175

Please sign in to comment.