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

document merge-based approach to updating existing PRs #2470

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
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
42 changes: 42 additions & 0 deletions PRs.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,45 @@ Look at the PR and create it if it looks good.
# This example's PR was at:
https://github.com/apache/lucene/pull/2
```
3. Instead of rebasing, you can `git merge` the upstream `main` branch from the new
project into your existing PR branch, push the result to your own fork of the new
project, and use that as the basis for creating a PR against the new upstream project.

This approach works cleanly because the `main` branch on each of the new projects is
a direct descendant of the merge-base (with `lucene-solr/master`) of all existing
PR branches against the legacy joint `lucene-solr` project.

A benefit of a `merge`-based approach (as opposed to rebasing or applying a patch) is
that commit history (including commit hashes) is preserved, and remains compatible
across projects (and in some multi-commit cases, a merge-based approach can also
avoid the need to "re-resolve" related conflicts in multiple rebased commits).

NOTE: PRs updated in this way, if merged back into the `main` branch, could result
in an undesirably convoluted (if "correct") commit history. In many cases it may be
preferable to "squash-merge" such PRs (already a common general practice for merging
feature branches in these projects).

```
# clone new upstream project (optionally supplying remote name "apache" instead
# of "origin")
git clone --origin apache https://github.com/apache/lucene.git
cd lucene
# in Github UI, create [user]'s fork of new project (as in the "rebase" example)
# add [user]'s fork of the new project
git remote add mynewfork https://github.com/[user]/lucene.git
git fetch mynewfork
# add [user]'s fork of the legacy (joint) project
git remote add mylegacyfork https://github.com/[user]/lucene-solr.git
Copy link
Contributor

Choose a reason for hiding this comment

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

I like git merge and have adapted my remote fork naming convention to suit the repo split situation.

old convention

git remote add github_$user https://github.com/$user/lucene-solr

new convention

git remote add github_$user             https://github.com/$user/lucene
git remote add github_$user-lucene-solr https://github.com/$user/lucene-solr

Just sharing in case that might suit others too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @cpoerschke! I definitely prefer your $user indication of variable-substitution (and have adopted it in e6c9c2d).

Locally I have adopted an analogous (but slightly different) remote-naming convention. I know you weren't necessarily suggesting a change to this PR, but fwiw I think as far as the PR goes though, sticking with trivial names (mynewfork, mylegacyfork) simplifies things and helps keep the focus on the process per se. This also may help avoid creating the impression that there's an "official" recommended convention (a fact which may be obvious to many readers, but perhaps not all)?

git fetch mylegacyfork
# get the legacy PR's branch:
git checkout --no-track mylegacyfork/LUCENE-XXXX -b LUCENE-XXXX
# merge upstream main branch (a higher `merge.renameLimit` allows `git merge` to
# complete without the warning that it would otherwise print due to the large
# number of files deleted across the TLP split)
git -c merge.renameLimit=7000 merge apache/main
# after resolving any conflicts and committing the merge,
# push to [user]'s new fork
git push -u mynewfork LUCENE-XXXX
# in Github UI, create PR from mynewfork/LUCENE-XXXX (against apache/main)
# as in the "rebase" example
```