You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/03/18 21:37:11 UTC
[GitHub] [lucene-solr] magibney opened a new pull request #2470: document merge-based approach to updating existing PRs
magibney opened a new pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] cpoerschke commented on a change in pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#discussion_r597920457
##########
File path: PRs.md
##########
@@ -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
Review comment:
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] epugh commented on pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#issuecomment-802342499
so this may make sense at the root of `lucene-solr` to catch eyes, however I'd be interested in the same information over in `solr\dev_docs\` as well. Increase chances we find it in both places ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] epugh commented on pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#issuecomment-802330482
I will give this a test this evening.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] janhoy commented on pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
janhoy commented on pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#issuecomment-802794377
LGTM, did not test it..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] magibney commented on a change in pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
magibney commented on a change in pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#discussion_r598724131
##########
File path: PRs.md
##########
@@ -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
Review comment:
Thanks, @cpoerschke! I definitely prefer your `$user` indication of variable-substitution (and have adopted it in e6c9c2d5).
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)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] magibney commented on pull request #2470: document merge-based approach to updating existing PRs
Posted by GitBox <gi...@apache.org>.
magibney commented on pull request #2470:
URL: https://github.com/apache/lucene-solr/pull/2470#issuecomment-802986119
I'm inclined to leave this as-is for now (with the recent minor addition of the `merge.renameLimit` increase, which I think is probably mostly cosmetic wrt suppressing a warning that I think in this context is probably not significant? -- can't hurt, in any case). ... unless there are any modifications suggested.
My impression is that this is placed in the root of `lucene-solr` not so much to catch eyes as because this is really about the migration, for people who may still be looking mainly at the legacy joint project, and because it applies equally to Lucene- and Solr-focused PRs.
I could see placing a more general-purpose "guide to working with Solr PRs" markdown/adoc file in `solr:solr/dev-docs`, and maybe linking from there to this top-level legacy project "PR migration" readme? That way the content can be maintained in a single (most logical) place. However this is approached, perhaps it makes sense as a separate issue (in the new solr TLP)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org