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) > > >
