You can either A) rebase and add the "Closes #XX" message. In this case github with mark the PR as closed B) Merge the PR and don't rebase. If the hash you push matches the hash of the PR, then github will automatically mark the PR as merged.
What you did was rebase but not add the "Closes #XX" message. If you want to do (B), you have to be a little bit careful to make sure you don't accidently rebase if you have pull.rebase set to true. Just make sure the hashes match. -Dan On Fri, Jan 8, 2016 at 12:13 PM, Kirk Lund <[email protected]> wrote: > The latter confuses me. If I want to add "Closes #XX" to the PR's commit > message then I have to do a rebase before committing. How would merging it > without rebasing close the PR? Isn't that what I was doing that then > resulted in me having to issue an empty commit with "Closes #XX" after the > fact? > > > On Fri, Jan 8, 2016 at 12:10 PM, Dan Smith <[email protected]> wrote: > > > The way our github integration is set up with apache, only the original > > submitter of the PR can close the PR through the github API. Committers > can > > only close PRs by committing with "Closes #XX" in the message or merging > in > > the PR without rebasing. > > > > -Dan > > > > On Fri, Jan 8, 2016 at 9:46 AM, John Blum <[email protected]> wrote: > > > > > True, the graph definitely, but the commit messages, not so much. > > > > > > On Fri, Jan 8, 2016 at 9:38 AM, Kirk Lund <[email protected]> wrote: > > > > > > > That's probably caused by the fact that many of us are just learning > > git. > > > > > > > > -Kirk > > > > > > > > > > > > On Fri, Jan 8, 2016 at 9:34 AM, John Blum <[email protected]> wrote: > > > > > > > > > I guess the only reason I mention it is the Apache Geode commit > > history > > > > is > > > > > a mess (inconsistent, in many cases, no correlation to the > changelog > > or > > > > > JIRA tickets, etc)... > > > > > > > > > > Running a git log -v --graph also illustrates another problem (a > > > > non-linear > > > > > series of commits cause by not rebasing, which ought to be allowed > on > > > > > "topic" branches). > > > > > > > > > > $0.02 > > > > > -John > > > > > > > > > > > > > > > On Fri, Jan 8, 2016 at 9:27 AM, Kirk Lund <[email protected]> > wrote: > > > > > > > > > > > The problem in this case is that the changes for the PR were > > > committed > > > > > > without "Closes #38" so that PR remains open. I don't have > > > permissions > > > > on > > > > > > https://github.com/apache/incubator-geode to close any PRs > > manually. > > > > The > > > > > > only way I know of to close them is via a commit that includes > > > "Closes > > > > > #38" > > > > > > in the commit message and then the asfgit bot closes it for us. > > > > > > > > > > > > -Kirk > > > > > > > > > > > > > > > > > > On Fri, Jan 8, 2016 at 9:17 AM, John Blum <[email protected]> > > wrote: > > > > > > > > > > > > > I just clarify, when you push the "patch" associated with the > PR > > > (if > > > > > done > > > > > > > properly) it will automatically close the PR. If not done > > > properly, > > > > > then > > > > > > > you can manually close it without a commit. > > > > > > > > > > > > > > On Fri, Jan 8, 2016 at 9:16 AM, John Blum <[email protected]> > > > wrote: > > > > > > > > > > > > > > > You don't need to push commits to close PRs (at least not in > > > > GitHub; > > > > > > not > > > > > > > > sure how Apace works). > > > > > > > > > > > > > > > > On Fri, Jan 8, 2016 at 9:14 AM, Kirk Lund <[email protected]> > > > > wrote: > > > > > > > > > > > > > > > >> Since #36 and #38 were already merged into develop via #42, > > > > should I > > > > > > > >> closed > > > > > > > >> them with two separate empty commits or is there a way to > > > combine > > > > > > them? > > > > > > > >> > > > > > > > >> git commit --allow-empty -m "Closes #36 *Already fixed*" > > > > > > > >> git commit --allow-empty -m "Closes #38 *Already fixed*" > > > > > > > >> > > > > > > > >> -Kirk > > > > > > > >> > > > > > > > >> > > > > > > > >> On Thu, Jan 7, 2016 at 5:13 PM, Dave Barnes < > > [email protected] > > > > > > > > > > wrote: > > > > > > > >> > > > > > > > >> > Update index.html > > > > > > > >> > #38 opened on Nov 18, 2015 by GregChase > > > > > > > >> > PR #38 should be closed. I merged #38 with #36 into a > later > > > pull > > > > > > > >> request, > > > > > > > >> > #42, which was committed as part of the web page update. > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > On Thu, Jan 7, 2016 at 4:48 PM, Dan Smith < > > [email protected]> > > > > > > wrote: > > > > > > > >> > > > > > > > > >> > > #29 caused test failures. I commented on that and I was > > > hoping > > > > > the > > > > > > > >> author > > > > > > > >> > > would pick that up and fix the failures, otherwise we > may > > > want > > > > > to > > > > > > > fix > > > > > > > >> > those > > > > > > > >> > > and merge that at some point. > > > > > > > >> > > > > > > > > > >> > > -Dan > > > > > > > >> > > > > > > > > > >> > > On Thu, Jan 7, 2016 at 4:41 PM, Kirk Lund < > > [email protected] > > > > > > > > > > wrote: > > > > > > > >> > > > > > > > > > >> > > > We have 6 pull requests that have been open for quite > a > > > > while. > > > > > > Is > > > > > > > >> > someone > > > > > > > >> > > > already working on each of these? What's the status on > > > them? > > > > > > > >> > > > > > > > > > > >> > > > https://github.com/apache/incubator-geode/pulls > > > > > > > >> > > > > > > > > > > >> > > > Enabling direct reporting on Geode's website > > > > > > > >> > > > #66 opened 10 days ago by rvs > > > > > > > >> > > > > > > > > > > >> > > > GEODE-341/ GEODE-628: Refactor Java packages to > reflect > > > > Apache > > > > > > > >> > > organization > > > > > > > >> > > > /Rename container folder to "geode-jvsd" > > > > > > > >> > > > #49 opened on Dec 8, 2015 by jujoramos > > > > > > > >> > > > > > > > > > > >> > > > Verified preceding content merges, fixed a couple of > > > typos. > > > > > > > >> > > > #47 opened on Dec 4, 2015 by davebarnes97 > > > > > > > >> > > > > > > > > > > >> > > > Addresses the documentation component of GEODE-268, > > adding > > > > > > > >> > explanatio... > > > > > > > >> > > > #43 opened on Nov 23, 2015 by davebarnes97 > > > > > > > >> > > > > > > > > > > >> > > > Update index.html > > > > > > > >> > > > #38 opened on Nov 18, 2015 by GregChase > > > > > > > >> > > > > > > > > > > >> > > > GEODE-252] Remove deprecated PartitionAttributes > methods > > > > > > > >> > > > #29 opened on Nov 5, 2015 by shroman > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > -John > > > > > > > > 503-504-8657 > > > > > > > > john.blum10101 (skype) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > -John > > > > > > > 503-504-8657 > > > > > > > john.blum10101 (skype) > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > -John > > > > > 503-504-8657 > > > > > john.blum10101 (skype) > > > > > > > > > > > > > > > > > > > > > -- > > > -John > > > 503-504-8657 > > > john.blum10101 (skype) > > > > > >
