Do not squash commits #1723
Merged
Do not squash commits #1723
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Squashing commits may be helpful for small pull requests where the total change is around 10—20 lines or two or three hunks. With such a small patch it's easy to infer intent in the future. For larger patches it becomes increasingly difficult to determine why the change was made when a single commit contains dozens of hunks where none of them are related. This is especially important after either: * Enough time has passed that you don't remember why you made the patch * The patch was made by a maintainer that is no longer active * The patch was made by a contributor that is no longer interested In these situations, when an issue is found related to the patch the person working in the area has to infer the purpose of the patch from the context they have. The new work won't necessarily be a bug in the original patch. It may add functionality that incidentally became an important feature for some of our users, and the new work wishes to change something related to that change. The new work may be a bug fix that has a subtle interaction, so determining if the first patch was a buggy (even if unintentionally so) change, or more importantly how the change is buggy, will be difficult to determine the less context we have. By aggressively squashing commits we remove the commit messages that contain that very important context. I see no reason to forbid squashing commits in a way that makes the intent of the patch more clear. On the other hand, encouraging it via a check-box on our PR form may cause the loss of this important context when we need it.
|
👍🏻 from me |
|
@homu r+ |
|
📌 Commit f22239e has been approved by |
homu
added a commit
that referenced
this pull request
Sep 24, 2016
Do not squash commits # Description: Squashing commits may be helpful for small pull requests where the total change is around 10—20 lines or two or three hunks. With such a small patch it's easy to infer intent in the future. For larger patches it becomes increasingly difficult to determine why the change was made when a single commit contains dozens of hunks where none of them are related. This is especially important after either: * Enough time has passed that you don't remember why you made the patch * The patch was made by a maintainer that is no longer active * The patch was made by a contributor that is no longer interested In these situations, when an issue is found related to the patch the person working in the area has to infer the purpose of the patch from the context they have. The new work won't necessarily be a bug in the original patch. It may add functionality that incidentally became an important feature for some of our users, and the new work wishes to change something related to that change. The new work may be a bug fix that has a subtle interaction, so determining if the first patch was a buggy (even if unintentionally so) change, or more importantly how the change is buggy, will be difficult to determine the less context we have. By aggressively squashing commits we remove the commit messages that contain that very important context. I see no reason to forbid squashing commits in a way that makes the intent of the patch more clear. On the other hand, encouraging it via a check-box on our PR form may cause the loss of this important context when we need it. ______________ # Tasks: - [x] Describe the problem / feature - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md).
|
☀️ Test successful - status |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Squashing commits may be helpful for small pull requests where the total
change is around 10—20 lines or two or three hunks. With such a small
patch it's easy to infer intent in the future.
For larger patches it becomes increasingly difficult to determine why
the change was made when a single commit contains dozens of hunks where
none of them are related.
This is especially important after either:
In these situations, when an issue is found related to the patch the
person working in the area has to infer the purpose of the patch from
the context they have. The new work won't necessarily be a bug in the
original patch. It may add functionality that incidentally became an
important feature for some of our users, and the new work wishes to
change something related to that change.
The new work may be a bug fix that has a subtle interaction, so
determining if the first patch was a buggy (even if unintentionally so)
change, or more importantly how the change is buggy, will be difficult
to determine the less context we have.
By aggressively squashing commits we remove the commit messages that
contain that very important context. I see no reason to forbid
squashing commits in a way that makes the intent of the patch more
clear. On the other hand, encouraging it via a check-box on our PR form
may cause the loss of this important context when we need it.
Tasks:
I will abide by the code of conduct.