> Le 19 juin 2017 à 05:14, Jaikiran Pai <jai.forums2...@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/> > The other thing I had in mind, if we agree upon, is to have an enhancement > raised with the ASF infra team to allow adding some specific comment on the > open PR by a *committer* which then auto-closes the PR. Some comment like > “This PR is merged”. I don’t think the ASF infra team can do something about it. The « This closes #x » is something that is supported by every github repository, this is not specific to the mirrored ASF repos. Probably a simpler and better integration with github would be to make the PR tracker editable to ASF committers. And probably both github and asf teams have heard about that. Nicolas
