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

Conversation

magibney
Copy link
Contributor

No description provided.

@epugh
Copy link
Contributor

epugh commented Mar 18, 2021

I will give this a test this evening.

@epugh
Copy link
Contributor

epugh commented Mar 18, 2021

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 ;-)

@janhoy
Copy link
Contributor

janhoy commented Mar 19, 2021

LGTM, did not test it..

@magibney
Copy link
Contributor Author

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)?

PRs.md Outdated
Comment on lines 92 to 95
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)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants