It’s also possible to git revert <sha1-of-merge-commit> – you just have to use the -m option to tell git which parent you’re reverting relative to.
@jerith Another git question for you.
I want all patches on say elm/core to be based off the latest release, which is 1.0.5. 1.0.5 is what I will be building the patch stack on top of to form a release branch. In many cases, PRs may have been created by starting from the elm/core master branch, which may have commits on it that came after 1.0.5.
If I merge such a PR back into my patch stack branch, won’t it also merge in those other commits from elm/core master? How do you detect that this will happen, and only merge the patch from the PR? I never tried rebasing onto an earlier point in the commit history before, does it work?
Tried a little experiement with git to confirm this happens.
master → created text.txt with “master1” in it.
branch1 off master → added line “branch1” to test.txt
branch2 off branch1 → added line “branch2” to test.txt
merged branch2 onto master, now test.txt contains the lines “branch1” and “branch2”.
So I created a GitHub org, and Dillon made a new channel on the Discord server for the name elm-janitor:
To get on the discord server, I created an invite link.
6 Likes
Hi Rupert, I’m not jerith, but I hope it’s ok if I answer your question: in this case, you shouldn’t merge, but rebase. You can test it in your example with another branch:
git branch rebasedBranch2 branch2
git rebase --onto master branch1 rebasedBranch2
Now “rebasedBranch2” should be based on “master” and contain only the commits between “branch1” and “branch2”. In the next step you could merge the new “rebasedBranch2” into “master”.
Years ago when we switched to git at work, I completed the following interactive tutorial: https://learngitbranching.js.org. It was fun and helped me a lot working with git…
2 Likes
Thanks. So if I have a PR checked out, to rebase it onto the current release:
git rebase --onto 1.0.5 master
This seems to work no matter where the PR was originally branched from. If it was before the release, it rebases it up to there. If it was after the release, it brings it back. If its already in the right place, you just get a message saying its already up to date.
Yep, rebasing is the way to avoid sucking in extra commits from upstream master.
I would suggest you ask the PR author to do this rather than doing it yourself. That way the author can open a PR with the exact same (rebased) commits against the upstream repo as well, which will ease future integrations.
1 Like
Touches on a few things I have been thinking about.
In general, best to contact the original PR author to ask for changes. That works well from the inclusivity/involvement point of view too. I think as some PRs have been hanging around for a while there might not be a response in some cases - so I probably need to record “contacted author on <date>” somewhere, and mark as no response in say 2 weeks.
Its not wrong to submit a PR based off of latest master, or some might even be based off of older releases than the current one. So should I really ask the original author to rebase it? Almost all of the time rebasing it will work without conflicts, and the patch is not changed at all by doing that. Git will still track whether it is merged or not. So I think I will rebase into a local branch on the fork (called pr-123 or whatever its number is), if that is conflict free then ok to continue without asking original author to do that part.
If there are conflicts, need to reformat code, correct some small mistake, or reorganize the commits a bit better - ask the the original author to do that part.
That leaves what to do if the original author does not do the work needed to get the PR in shape. I guess clean it up in a local branch, then submit that back as a new PR, and merge that patch in from the new PR.
Finally, is there some convention I can follow so that I can really easily get a list of all the PRs that have been merged? Maybe by ensuring that the Github PR number is added to the commit messages, like #123. Some other way that would work better?
That all sounds right to me. Though note that a PR based off of an older release is not necessarily a problem – most of the time it’ll still merge cleanly without needing to be rebased on top of the latest release.
Finally, is there some convention I can follow so that I can really easily get a list of all the PRs that have been merged?
At first I wasn’t sure what you meant here (since GH will trivially tell you which PRs you’ve merged and which you haven’t). But I think you’re asking about tracking the upstream PRs, right? E.g. that elm/core PR #123 corresponds to elm-janitor/core PR #456? Esp. if the commit you’re merging in uses the common GH convention of including a line like fixes #123 … but which #123 in which fork?!
Ah, I found this GH help page: So for my example above, you could do this in the merge commit message:
fixes elm/core#123
fixes elm-janitor/core#456
Then something like git log | grep ^fixes elm/ would give you a list of all upstream PRs that you have fixes for.
1 Like
Great that looks like it will work. Thanks.
I don’t think “fixes” is what you want there right? That’s for auto-closing issues. Sounds like you just want to relate PRs together. So something like “Corresponds to elm/core#123” would make more sense at a glance.
1 Like
I think “fixes” is right – if this same commit were merged into elm/core's master branch, it would fix that issue. Auto-closing won’t happen unless/until that merge happens – merging it into elm-janitor/core's master branch will only auto-close the elm-janitor/core issue.
I see. I thought elm/core#123 in your example was a PR because you were talking about connecting PRs. I’m not following why there would even be issues in elm-janitor/core, at least not issues that duplicate the ones in elm/core.
You’re right, I was definitely conflating PRs and issues in my last two posts! And you’re also right that it probably doesn’t make sense to have issues in elm-janitor/core – just PRs.
So let’s say we have issue elm/core#123, which has a proposed fix in PR elm/core#789, and that same fix is also submitted in PR elm-janitor/core#456. Then I think all that’s needed is fixes elm/core#123 in the commit message. I believe that’ll cause GH to auto-generate a note in the root issue elm/core#123 that references the elm-janitor/core#456 PR, because that PR’s text (taken from the commit message) will mention the issue.
That’s the key thing in my mind: people who run into an Elm bug are gonna find the original issue in the elm/* project, and if GH auto-generates that link, they’ll be able to make their way over to the elm-janitor PR and find instructions for how to make use of it in their project.
For example, I mentioned the elm/core#1018 PR in a commit on my project, and GH automatically created a link from the former to the latter. Admittedly the thing in elm/core was a PR, not an issue, but I’m pretty sure this mechanism works for either.
1 Like
Yep I agree. 
If a PR also exists in elm/core (and I assume/hope that will always be the case) it may also make sense to reference the PR as well so that there’s a direct breadcrumb from there.
But I suppose we’re down to minutiae that can be documented and iterated on in a project wiki.
Thanks for your thoughts on it. I am thinking it might make sense to turn PRs off for elm-janitor packages - to encourage the PR to be made against the official version first? Not sure if GitHub lets me turn off PRs without also turning off Issues though, I should check that.
Not quite complete yet, but I have been trying to document a process (and FAQ type stuff) here:
https://github.com/elm-janitor/manifesto/blob/master/README.md
https://github.com/elm-janitor/manifesto/blob/master/git-help.md
Ah, that is interesting. I didn’t realize you could turn off PRs.
Well, I think turning off issues would be good too so those live in the official repos.
I was thinking leave them available - not so much for issues on the core package code, but someone might have some other random issue they want to mention. I think GitHub lets you set some text to guide people when entering issues, so can put a statement there directing them to the official repo if its a code issue.
This topic was automatically closed 10 days after the last reply. New replies are no longer allowed.