If it is not easy to add JDK11 support for the PreCommit-HBASE-Build job then I'm +1 with stopping it. We should make all the contribution ways have the same pre commit check.
Nick Dimiduk <[email protected]> 于2020年2月25日周二 上午7:33写道： > > The bot is for lots of projects, not only HBase, so we can not shut it > down. > > Pardon me Duo. I am proposing we terminate PreCommit-HBASE-Build. The > PreCommit-Admin job appears to be the responsibility of Yetus and is out of > scope of my discussion here. > > > 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. > > Sean is calling me out here, and it's correct. > > I have recently been looking to add JDK11 support to all of our CI jobs > (HBASE-23875 and friends), and Jira-attached patch files is the one that > demands the most work, just to get up to be on par with the rest. See > HBASE-23874 and HBASE-23879 as prerequisites.... And then today I uncovered > this little gem: it looks like PreCommit-HBASE-Build is broken for all but > whatever is the first attachment file recognized as a patch. > https://issues.apache.org/jira/browse/HBASE-23888 > > Final nail in the coffin is that it seems the global PreCommit-Admin job is > not actually running on it's intended 10-minute interval. It broke today > and no one noticed until I went looking. When it does run, it's somehow > missing newly attached files. This is evident by way of the output of the > last couple job runs missing Issue/Attachment pairs for the various > attachments on HBASE-23874. > > So yes, I do believe that a PR results in a better code review. I'm also > selfishly interested in cutting work that we don't *have* to do, supporting > a process that isn't used by the majority of our contributors. > > On Sat, Feb 22, 2020 at 6:52 AM Sean Busbey <[email protected]> wrote: > > > 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 > > > > > > > > > > > > > > >
