[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ant-dev
Subject:    Re: Process for handling GitHub PRs and closing them
From:       Matt Sicker <boards () gmail ! com>
Date:       2017-06-19 16:11:15
Message-ID: CACmp6kpLD59qtvSci4b+0pSsJqUU58+u1P8oLDde51Hy-fkF5g () mail ! gmail ! com
[Download RAW message or body]


What I've done on other Apache projects is essentially "git pull
github/pull/N/head", merging directly into master (this is after it's ready
to merge of course). After pushing that, GitHub will see that the branch is
merged and auto-closes the PR. If you rewrite history as with the rebase
example, GitHub (and git in general) doesn't notice that it's merged, so it
doesn't auto-close. If you really want to rebase, there's no way around
adding an additional commit to add the magic "closes #N" message.

On 19 June 2017 at 09:48, Jaikiran Pai <jai.forums2013@gmail.com> wrote:

>
> On 19-Jun-2017, at 2:43 PM, Nicolas Lalevée <nicolas.lalevee@hibnet.org>
> wrote:
>
> >
> >> Le 19 juin 2017 Ã  05:14, Jaikiran Pai <jai.forums2013@gmail.com> a
> écrit :
> >>
> >> We have (read only) github repos which back our main ASF git repos
> (consider the github ant-ivy repo which is a read-only mirror of ASF git
> repo). Users submit pull requests to our github repos and the process I
> follow for merging such PRs is the "rebase" approach which looks something
> like this:
> >>
> >>
> >> - Fetch the PR locally (git fetch github pull/45/head:pr-45)
> >> - Checkout to that branch locally (git checkout pr-45)
> >> - Rebase that PR on top of latest ASF (upstream) repo (git rebase
> asf/master)
> >> - Run a short build, verify and push to ASF repo (git push asf
> pr-45:master)
> >>
> >> (assume 45 is the pull request id).
> >>
> >> So essentially, I rebase the commits from the PR on top of the latest
> master and then push to the ASF repos. All this works fine and the ASF
> repos get those changes. However, this doesn't "close" the pull request on
> github.
> >>
> >> Apparently, the way to have the pull request closed is doing a actual
> "merge" of the pull request commits into the ASF repo instead of rebasing
> the commits.
> >>
> >> Then the other approach, which isn't that clean IMO, is to push a
> commit to the ASF repo with a commit message which includes "This closes
> #X" where X is the pull request id. The ASF github bot then notices this
> commit messages and goes and closes the open PR.
> >>
> >> I usually prefer the rebase approach (the one I outlined above) for
> dealing with pull requests, since it gives a clean git commit tree. But
> clearly that doesn't have a way to close the PR.
> >>
> >> Is there any preferred approach that we should follow with PRs?
> >
> > I agree with the approach you have. I have been lazy though, I just use
> the command line suggested in the email notifications, which makes things
> quite straight forward.
> >
> > I would just insist on trying to get the PR closed. Even if it may
> pollute the commit log, we should put the "This closes #X" message. And it
> can be viewed as a marker, just like we do with Jira. Also, we could make
> thing more explicit by specifying the full path of the github project:  «
> closes apache/ant-ivy#123  ».
> > See the github documentation about it: https://help.github.com/
> articles/closing-issues-via-commit-messages/ <https://help.github.com/
> articles/closing-issues-via-commit-messages/>
> >
>
>
> I wasn't aware that this kind of commit message comes from and is
> supported by github itself. I was under the impression that this is
> something that the ASF github integration had introduced. Thanks for that
> link.
>
> What you suggest about making sure that the PR gets closed even if we end
> up with those mark commit messages, sounds fine to me and I'll start doing
> that.
>
> -Jaikiran
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>


-- 
Matt Sicker <boards@gmail.com>


[prev in list] [next in list] [prev in thread] [next in thread]