Created HBASE-21701 'Accept pull requests from contributors' and added to a comment what was collected until now. Feel free to add more there!
On Wed, Jan 9, 2019 at 11:22 AM Tamas Penzes <tam...@cloudera.com.invalid> wrote: > +1 for rebase and merge. > > It is the most understandable merge strategy. Others can cause huge > confusion when checking the history. > > On Wed, Jan 9, 2019, 00:19 Ankit Singhal <ankitsingha...@gmail.com wrote: > > > Just sharing what other communities are thinking on some of the merging > > strategies provided by github for pull requests:- > > > > > https://lists.apache.org/thread.html/c41aef9a33548de8e543d01611e71316c1cd0b0d0a75fb583d609ae1@%3Cdev.calcite.apache.org%3E > > > > "default merge pull request" option:- > > Affects the linear history of the commits , it will be hard to follow > which > > is commit recently. > > https://help.github.com/articles/about-pull-request-merges/ > > > > "Squash merge" option:- > > Calcite community is considering of disabling the feature from Github and > > delegating it to the author to squash all their commit before it can be > > merged by the committer so that original author of the PR can be > preserved > > in the squashed commit. > > > > "rebase and merge" option:- > > This is how most of the apache projects currently doing through git > client, > > It will preserves the author and linear history of the commit.(also > tested > > by calcite community and said on below blog) > > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/ > > > > So , we may should consider disabling the ones which doesn't suit us to > > avoid committers using them accidentally. > > > > Regards, > > Ankit Singhal > > > > > > On Tue, Jan 8, 2019 at 11:07 AM Peter Somogyi <psomo...@apache.org> > wrote: > > > > > I believe we reached consensus to migrate our remaining repositories to > > > GitBox before the mandatory migration which will happen on 7th of > > February. > > > Apart from 'hbase' we still have 'hbase-site' and 'hbase-thirdparty' > > > repositories that also require the same migration process. > > > > > > From users' point of view they could still use git:// > > > git.apache.org/hbase.git for read only access, only committers need to > > > change the remote URL to the GitBox one. Jenkins jobs are already using > > the > > > GitHub URL for cloning the repository and I created a patch for the > > > documentation and website changes in HBASE-21685 that we can merge > after > > > the process is competed. > > > > > > There's still outstanding work to do before we have good guidelines on > > > accepting pull requests on GitHub, but the GitBox migration doesn't > > require > > > our committers to start working with PRs in a different way. > > > > > > If there is no disagreement I'd kindly ask one of the PMC members to > > reach > > > out to INFRA to perform the migration. > > > > > > Thanks, > > > Peter > > > > > > On Sun, Dec 9, 2018 at 12:56 AM Andrew Purtell < > andrew.purt...@gmail.com > > > > > > wrote: > > > > > > > Sounds good to me except squash merge at commit of PR shouldn’t be an > > > > option it should be required except for good reason (and I don’t know > > > what > > > > that would be ) > > > > > > > > > On Dec 8, 2018, at 3:28 PM, Stack <st...@duboce.net> wrote: > > > > > > > > > >> On Fri, Dec 7, 2018 at 6:23 PM Sean Busbey <bus...@apache.org> > > wrote: > > > > >> > > > > >> The move to gitbox doesn't require us to only accept github PRs. > > Given > > > > >> the current rate of contributions via patches on JIRA vs GitHub > > PRs, I > > > > >> wouldn't want to push for that now. > > > > >> > > > > >> The change does make it easier for us to start encouraging PR > > > > >> submissions, because committers will be able to directly merge > from > > > > >> the GitHub UI. > > > > >> > > > > >> I'd recommend we try to keep this as a small incremental change. > > That > > > > >> would mean: > > > > >> > > > > >> * committers ensure there's an associated JIRA for release note > and > > > > >> precommit checks (that can be just by pinging the contributor to > go > > > > >> make one) > > > > >> * backports are still handled by the committer if they're simple > and > > > > >> the contributor if there's a problem. I think saying "open a new > PR > > to > > > > >> backport to branch FOO" is perfectly reasonable since it's > analogous > > > > >> to when we ask contributors to attach a branch specific patch. > > > > >> * committers ensure the pushed commit has a message that follows > our > > > > >> current practice (which would mean looking out for the "helpful" > > > > >> subject wrapping) > > > > >> * Squash merge is an option when the committer goes to accept the > > PR. > > > > >> the contributor is free to either push additional commits or > squash > > on > > > > >> their branch when working through reviews, I don't think we need > to > > > > >> weigh in on how contributors choose which of those works best for > > > > >> them. > > > > >> > > > > >> That way we can also incrementally improve how well we handle PR > > > > >> submissions by better documenting expectations and building up > > > > >> additional tooling (e.g. having our precommit feedback go directly > > to > > > > >> the PR instead of being tied to a JIRA) > > > > >> > > > > > > > > > > This seems reasonable to me. Andrew's strawman would be too > radical a > > > > > change. > > > > > Thanks, > > > > > S > > > > > > > > > > > > > > >>> On Fri, Dec 7, 2018 at 12:09 PM Stack <st...@duboce.net> wrote: > > > > >>> > > > > >>>> On Fri, Dec 7, 2018 at 9:03 AM Sean Busbey <bus...@apache.org> > > > wrote: > > > > >>>> > > > > >>>> Hi folks! > > > > >>>> > > > > >>>> Per the email from infra "[NOTICE] Mandatory relocation of > Apache > > > git > > > > >>>> repositories on git-wip-us.apache.org" ( > > https://s.apache.org/0sfe > > > ) > > > > >> it > > > > >>>> looks like the future of interacting directly with PRs is coming > > > > sooner > > > > >>>> rather than later. > > > > >>>> > > > > >>>> I think we should move to gitbox ASAP rather than wait for the > > > crunch. > > > > >> If > > > > >>>> we hit a rough spot we're more likely to get some help when > things > > > > >> aren't > > > > >>>> busy. Maybe we wait until our open RCs close so that folks that > > need > > > > >> to tag > > > > >>>> those releases don't need to update their workflow first? > > > > >>>> > > > > >>>> Presuming everyone still agrees that we get value out of JIRA, I > > > think > > > > >> we > > > > >>>> need update our committer guidelines to expressly remind folks > to > > > > >> check on > > > > >>>> things like commit messages before merging PRs, as well as to > > ensure > > > > >> folks > > > > >>>> use the "squash and merge" option to keep the git history less > > > > >> complicated. > > > > >>>> Probably a good time to add text about the importance of > > > backporting, > > > > >> since > > > > >>>> there isn't a github UI for doing that. > > > > >>>> > > > > >>> > > > > >>> > > > > >>> Sounds good. > > > > >>> > > > > >>> Use this thread to list what needs documentation? (Agree with the > > > "Need > > > > >> to > > > > >>> sort all of this out and provide clarity now before a switch > over." > > > > from > > > > >>> Andrew). > > > > >>> > > > > >>> What should the commit be like? Should be like now? What about > that > > > > ugly > > > > >>> bleed that happens when the first line is too long and gets > dumped > > > into > > > > >> the > > > > >>> textbox below ... which then becomes the log IIRC. > > > > >>> > > > > >>> When do we do the squash merge? Is that the committer who does > this > > > > after > > > > >>> rounds of review? > > > > >>> > > > > >>> I like Andrew's list. > > > > >>> > > > > >>> On the 'You can't fix a branch-1 issue where the code is > different > > in > > > > >>> branch-2 and up by opening a PR against master', this is a prob. > at > > > > least > > > > >>> with our current 'process'. We don't do a JIRA per push because > it > > is > > > > >> just > > > > >>> a bunch of busy work. Do we have to do this now (any > alternatives?) > > > > >>> > > > > >>> Thanks for starting this up Sean, > > > > >>> S > > > > >> > > > > > > > > > >
