Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guidance on making commits atomic #696

Merged
merged 3 commits into from
Mar 22, 2022
Merged

Add guidance on making commits atomic #696

merged 3 commits into from
Mar 22, 2022

Conversation

chao-xian
Copy link
Contributor

@chao-xian chao-xian commented Mar 21, 2022

Most developers are very good at making their commits granular/atomic. But it takes practise and isn't currently our recommended way. It should be something that we strive for.

It's really long, and uses H5 headings. Any new section would result in unreadable H6s.
@chao-xian chao-xian force-pushed the commit-style branch 2 times, most recently from 3c2f684 to 0017f21 Compare March 21, 2022 20:10
@chao-xian chao-xian marked this pull request as ready for review March 21, 2022 20:15
@injms
Copy link
Contributor

injms commented Mar 22, 2022

I think atomic commits are very useful, especially in how it helps makes reviewing clearer when stepping forwards and backwards through the commits whilst still having a (hopefully) working branch. I added a small suggestion to make the description of what an atomic commit is a bit clearer.

For easier reading.

Co-authored-by: Ian James <[email protected]>
Copy link
Contributor

@injms injms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super! Thanks also for tidying up the rest of the section 👍

@chao-xian
Copy link
Contributor Author

Thanks @injms!

@chao-xian chao-xian merged commit 3c69254 into main Mar 22, 2022
@chao-xian chao-xian deleted the commit-style branch March 22, 2022 11:28
@philandstuff philandstuff mentioned this pull request Mar 31, 2022
timblair added a commit that referenced this pull request May 25, 2022
Things were shuffled around in #696 but a couple of links got move.
Fixes #700.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants