Per this discussion, I have posted a patch on HBASE-25228 that removes the script we used to use that hooked up the Jenkins Precommit job to Jira. I have closed the above issues as "won't fix".
It also looks like our documentation needs an update, so I filed HBASE-25233. I think we might also be able to drop this "create-patch" script. Please take a look, and feel free to pick up any of this work that speaks to you. Thanks, Nick On Wed, Mar 4, 2020 at 9:50 AM Nick Dimiduk <ndimi...@apache.org> wrote: > > If it is not easy to add JDK11 support for the PreCommit-HBASE-Build job > then I'm +1 with stopping it. > > It's a bit of work to bring Jira PreCommit up to feature parity with > GitHub PreCommit, especially seeing the work that was necessary to get > GitHub PreCommit to do JDK11. From what I can tell, the work includes: > > HBASE-23879 Convert Jira attached patch precommit to a Jenkinsfile build > HBASE-23888 PreCommit-HBASE-Build ignores the `ATTACHMENT_ID` provided by > PreCommit-Admin > HBASE-23875 Add JDK11 compilation and unit test support to Jira attached > patch precommit > > All the work is in 23879 and 23888. Since I've already done the work for > JDK11 on GitHub PreCommit (HBASE-23767), 23875 should be fairly > straightforward. If we're going to keep both, we should add one more step, > which is to fold them both in the same `Jenkinsfile` and > `jenkins_precommit_yetus.sh` files. > > Since the above work is blocking the merge of JDK11 support for GitHub > PreCommit (HBASE-23767) and Nightlies (HBASE-23876), I'm going to disable > the Jira PreCommit job (rename it, so that PreCommit-Admin no longer find > it), and merge the JDK11 branches. If folks come back around missing the > Jira PreCommit, we can revive this discussion. > > Thanks, > Nick > > On Mon, Feb 24, 2020 at 5:38 PM 张铎(Duo Zhang) <palomino...@gmail.com> > wrote: > >> 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 <ndimi...@apache.org> 于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 <bus...@apache.org> 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) <palomino...@gmail.com> >> 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 <wellington.chevre...@gmail.com>于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, <apurt...@apache.org> >> > > 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 < >> ndimi...@apache.org> >> > > > > 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 >> > > > > > >> > > > > >> > > > >> > > >> > >> >
