ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jaikiran Pai <jai.forums2...@gmail.com>
Subject Re: Process for handling GitHub PRs and closing them
Date Mon, 19 Jun 2017 14:48:55 GMT

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


Mime
View raw message