There's a dedicated precommit job just for the hbase main repo. We can shut it down without impacting other projects or even other hbase repos. There's already debt on that job; Nick has been starting to pay it down and I'd imagine that's how we got to this thread about just turning it off. For example branch 1 handling is currently broken for the jira patch tester but not the GitHub PR tester.
I personally don't find it more difficult to evaluate either kind of contribution because I rely on Apache Yetus Smart Apply Patch which "just works" most of the time given just a jira key to grab either patch or PRs. However, I definitely agree with the much more difficult commenting workflow with just jira. Personally that means if my feedback is going to be more than "lgtm" then I ask the contributor to shift to a PR. I looked at runs of the jira patch precommit bit over Feb and most of them look like either duplicate work where it ended up testing a GitHub PR that the dedicated PR precommit also tested or an old patch getting retested due to age off of the "did I already test this" check of the coordinating job. There was a contribution from a first time contributor, but afaict they were following the ref guide rather than intentionally avoiding github. I'm also strongly in favor of just shifting to PRs. On Sat, Feb 22, 2020, 05:53 张铎(Duo Zhang) <[email protected]> wrote: > The bot is for lots of projects, not only HBase, so we can not shut it > down. > > My concern is that, do we really need to shut it down completely? The pre > commit job is still fine I think? So just leave it as is? And when > sometimes it is broken and needs a lot of effort to maintain, then we can > shut it down? > > Wellington Chevreuil <[email protected]>于2020年2月22日 > 周六19:12写道： > > > +1, can only see benefits of using GitHub PRs over attached patches. > > > > > > On Fri, 21 Feb 2020, 21:33 Andrew Purtell, <[email protected]> wrote: > > > > > +1 to the idea, having one contribution workflow instead of two is 50% > > less > > > confusing (or 100% depending how you count it). > > > > > > > applying a patch for local evaluation is more tedious than checking > > out a > > > PR branch. > > > > > > Except it's not necessary to download and apply the patch to evaluate > it, > > > we have precommit for both workflows. But that brings up something you > > > didn't mention, which is having two precommit workflows, one for JIRA, > > one > > > for PRs, can be burdensome. > > > > > > > > > On Fri, Feb 21, 2020 at 1:28 PM Nick Dimiduk <[email protected]> > > wrote: > > > > > > > Heya, and happy Friday. > > > > > > > > I would like to propose that we drop support for receiving > > contributions > > > by > > > > way of attaching a patch file to a Jira issue. From my perspective, > in > > > the > > > > face of modern interfaces for PR-style review, this is an "archaic" > > form > > > of > > > > contribution that is "actively harmful" to project health. I'm being > > > > intentionally divisive, but here's my argument. The "state of the > art," > > > > "modern" "best practices" revolve around a PR > > (GitHub/GitLab/Gerrit/&c.) > > > > -style review process that enjoys an intentionally designed user > > > experience > > > > and has many points of friction reduced or removed entirely. > > > > > > > > When a patch arrives via Jira attachment, it passes through a process > > > that > > > > suffers from a higher level of friction, something that actively > > > > discourages the level of code review that it could have received via > > PR. > > > I > > > > believe that reduced friction results is a more thorough, thoughtful > > > review > > > > and a review that's better able to handle large change-sets, vs. the > > > > attached patch process. Specific friction points include: > > > > * applying a patch for local evaluation is more tedious than > checking > > > out > > > > a PR branch. > > > > * each comment requires the reviewer to provide their own context > > > > (reference line numbers, copy-paste code snippets, &c) before saying > > > > anything at all. > > > > * threaded discussion around individual comments is impossibly > > difficult > > > > to manage for both participants and casual observers. > > > > * a super-human level of attention to detail is required on the > parts > > of > > > > both contributor and reviewer to ensure that all review comments have > > > been > > > > addressed. > > > > * syntax highlighting (while rudimentary vs. IDE-based evaluation) > > makes > > > > patches easier to read and digest. > > > > > > > > I claim "actively harmful" compared to the alternative because the > > above > > > > minor friction points, taken together, discourages the higher quality > > > > reviews that are possible by way of (1) the git-based interface to > the > > > > contributed content and (2) a commenting system that supports > > contextual, > > > > threaded, and resolvable comments. > > > > > > > > The primary counter argument I've come up with is based around user > > > access. > > > > It's possible that there's a contributor who has an Apache JIRA ID > but > > > not > > > > a GitHub ID, who is unwilling to make an account on the non-Apache > > > service. > > > > Not accepting issue-attached patches means we exclude that > contributor > > > from > > > > our community. However, I suspect that the number of active and > > potential > > > > contributors who falls into that bucket is at or approaching zero. I > > > > suspect that the world of potential contributors who have a GitHub ID > > but > > > > refuse to make an Apache JIRA ID is actually far greater. > > > > > > > > Thus I propose we discontinue accepting patches attached to Jira > > issues; > > > > our contributor guide would exclusively ask for a PR; we can shut > down > > > the > > > > pre-commit robot from scanning Jira. > > > > > > > > Thanks in advance for your thoughtful participation in this > discussion. > > > > Nick > > > > > > > > > > > > > -- > > > Best regards, > > > Andrew > > > > > > Words like orphans lost among the crosstalk, meaning torn from truth's > > > decrepit hands > > > - A23, Crosstalk > > > > > >
