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

Octopus merge strategy for rollups #34

Open
tgross35 opened this issue Aug 3, 2023 · 1 comment
Open

Octopus merge strategy for rollups #34

tgross35 opened this issue Aug 3, 2023 · 1 comment

Comments

@tgross35
Copy link

tgross35 commented Aug 3, 2023

I know this was mentioned somewhere on Zulip, but figured it's worth an issue to discuss.

It seems like using an octopus merge when handling >1 PR may be better than the current strategy of doing individual rollup commits. Some reasons:

  • It enforces the desired strategy for rollup merges, i.e. it will not allow a "complex merge that needs manual resolution". We already have to be sure that rollups contain only orthogonal branches, I have a mild suspicion that letting git handle the resolution may make this easier to implement.
  • Easier to understand history. It is currently kind of tricky to navigate around the git log and see (1) what was merged & tested together, and (2) what the source of the rollup is, since the rollup PR is not linked. Using a single merge commit to represent the rollup PR/test makes this easier to understand
  • More concise git history. This is obviously a low priority thing, but not having a separate merge commit for every PR helps keep history smaller and much less noisy
@tgross35
Copy link
Author

tgross35 commented Aug 3, 2023

If we want to keep the PR messages, we could put them all together into the merge commit.

That being said - I don't think there is much value to making the PR body exist in the git log. Linking to the PR in the merge commit is good, but the PRs themselves are often pretty bulky and contain information that isn't relevant. The commit messages really should be able to stand their own - but commit message quality is of course a separate conversation that does need to be had.

Related, there doesn't seem to be much value to including the branch name in the merge commit. A lot of times it's just noise like patch-1 that adds nothing of value, especially since the PR link handles this.

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

No branches or pull requests

1 participant