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.

Sign up for GitHub

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

Allow rebase strategy #7689

Merged
merged 3 commits into from
Aug 10, 2021
Merged

Allow rebase strategy #7689

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e85c47
Allow rebase strategy
Ryuno-Ki Aug 7, 2021
b2916c9
Typo fix
Ryuno-Ki Aug 9, 2021
2b7b04f
Remove: Any additional changes ...
hamishwillee Aug 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,11 @@ resolving them. You can do this by merging the `main` branch into your
branch (`git pull mdn main`), and then pushing the updated branch to
your fork (`git push`).

1. Once you've created your pull request, never use `git rebase` on your
branch if you need to make changes. Any changes should be made as
additional commits.
1. An alternative strategy is `git rebase` of `main` on your branch.
This will rewrite the git history and might confuse reviewers as notifications
from GitHub lead to nowhere. Your changes are replayed on top of the current
main branch at that point in time. Any additional changes should be made as
Copy link
Collaborator

Choose a reason for hiding this comment

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

I much prefer rebasing. As a reviewer it is rare to get lost in the historical rewrite, and rebase it creates a much better story for git archeologists.

Anyway, this is perhaps simpler?

While merging with the main branch using git pull is recommended (because it provides a more consistent git history), rebasing your changes using git rebase is also permitted.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In my experience, the merge strategy insofar, as it does provide an easier mental model.
For git bisect it's more challenging, since you will have commits with multiple parents.

My intention is to express, that rebase is „okay if you know what you're doing”.
I also noticed some stripped GPG verifications in my personal experience. That could also go into this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you've said actually captures "OK if you know what you're doing" because even if you know what you're doing you'll still have the highlighted issue (notifications leading nowhere). I prefer my text because it captures "rebase is allowed but merge is currently preferred".

Further, the "Any additional changes should be made as commits" jarred a bit. Either approach requires changes be made as commit - unless the point is to suggest don't do squash rebase (which is something I often do, and like).

It is completely up to you though - much easier to criticize than come up with text in the first place :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair point.
It's quite difficult to me to come up with a good phrasing (in a language that is not my first one) that „sounds good”.
Hence, I'm thankful for all the feedback you provide Hamish.

I will think more about how to put „rebase is allowed but merge is currently preferred” in elegant words.
Meanwhile: would the current state be good to go? Or shall I drop the „Any additional changes should be made as commits” part? (jarred is a word I would need to look up to remember its meaning, but I can understand the sense of it).

Squashing is a whole different subject. I only do it once I'm confident to not have to revert a commit.
But I feel, it would go to deep into git advanced knowledge at this point.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is hard to find exactly the right words. I've removed the "Any additional ..." bit and merged. Thank you!

commits.

1. Each pull request should contain a single logical change, or related set
of changes that make sense to submit together. If a pull request becomes
Expand Down