Re: [DISCUSS] Drop support for code contribution via Jira attached patch

[Nick Dimiduk <nd...@apache.org>]

> 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) <pa...@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 <nd...@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 <bu...@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) <pa...@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 <we...@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, <ap...@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 <
> ndimiduk@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
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] Drop support for code contribution via Jira attached patch

[Nick Dimiduk <nd...@apache.org>]

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 <nd...@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) <pa...@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 <nd...@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 <bu...@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) <pa...@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 <we...@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, <ap...@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 <
>> ndimiduk@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
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>