You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by "张铎(Duo Zhang)" <pa...@gmail.com> on 2022/03/15 13:17:32 UTC
[DISCUSS] Use spotless to auto format pom and java code
I've filed HBASE-26617 a while ago and recently I implemented a PR for
master branch.
https://github.com/apache/hbase/pull/4214
The PR is a bit large because it will also format the pom, the actual
changes are here
https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
In the PR I make use of the eclipse formatter and import order file to
format our java file. For pom, I just use most of the default formatter,
the only exception is to expand empty element.
The spotless plugin support setting a ratchetFrom option, which could be a
commit or a tag. Only files changed after this commit will be formatted, so
we can avoid generating a very big patch when running spotless:apply.
So after we get this in, before submitting a PR, you can just type mvn
spotless:apply, then most of the checkstyle issues will be solved
automatically. And we could also add a spotless:check step in our pre
commit job, to make sure we run spotless:apply before submitting a PR.
Just tell me what you think about the above plan. Suggestions are always
welcomed.
Thanks.
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Nick Dimiduk <nd...@apache.org>.
+1 for Duo's bump here -- let's bring this home!
On Tue, Apr 26, 2022 at 4:08 PM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> Just a reminder that some of us have almost reached a consensus on the
> final formatter config file.
>
> PTAL about the discussion and also the final style in this PR
> https://github.com/apache/hbase/pull/4312
>
> We have also talked about whether we should just use some well known styles
> such as google java format.
>
> So please reply here if you have any thoughts on this, i.e, which style do
> you prefer more, the current eclipse formatter, or google java format, or
> some other styles.
>
> Thanks a lot!
>
> 张铎(Duo Zhang) <pa...@gmail.com> 于2022年4月13日周三 07:48写道：
>
> > Some heads up, the current state on the big reformat can be reviewed here
> >
> > https://github.com/apache/hbase/pull/4312
> >
> > Shout if you have any questions about the formatter, we could try to
> tweak
> > the eclipse formatter to see if it could work as expected.
> >
> > Thanks.
> >
> > Andrew Purtell <ap...@apache.org> 于2022年4月3日周日 02:34写道：
> >
> >> I think we have consensus to move forward with application of the PRs,
> so
> >> let's fine tune the formatting configuration, and then regenerate and
> >> apply
> >> them.
> >>
> >>
> >> On Thu, Mar 31, 2022 at 11:03 PM 张铎(Duo Zhang) <pa...@gmail.com>
> >> wrote:
> >>
> >> > FWIW, you could do a 'git reset --hard' to the commit before the big
> >> > reformat and do a 'git blame' again on command line.
> >> >
> >> > Nick Dimiduk <nd...@apache.org> 于2022年4月1日周五 13:58写道：
> >> >
> >> > > I noticed that GitHub’s blame UI has a “show before this commit”
> >> feature,
> >> > > which allowed me to look at the blame info before the reformat was
> >> > applied.
> >> > > I assume the same is possible on the command line.
> >> > >
> >> > > On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com>
> >> > wrote:
> >> > >
> >> > > > +1
> >> > > >
> >> > > > Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
> >> > > >
> >> > > > > Now that I understand what 'ratchetFrom' in the plugin
> >> configuration
> >> > > was
> >> > > > > doing, I have removed it, and resubmitted the PRs. They are all
> >> > > > ridiculous
> >> > > > > and expected. Just about every file in the source is touched. On
> >> > master
> >> > > > > branch 4679 files are modified out of 5507 (excluding .git/).
> >> There
> >> > are
> >> > > > > similar results for other branches.
> >> > > > >
> >> > > > > I think we should first spot check the PRs to determine if
> >> spotless
> >> > > > > configuration should be fine tuned. Then once it is acceptable
> the
> >> > PRs
> >> > > > can
> >> > > > > be updated with a re-application. Merging them applies a one
> shot
> >> > > > > reformatting to all live branches. This is desirable so that
> going
> >> > > > forward
> >> > > > > it will be easier for reviewers and committers to manage PRs and
> >> > > commits
> >> > > > to
> >> > > > > multiple branches. Put another way, if we don't do it, merge
> >> > conflicts
> >> > > > are
> >> > > > > more likely.
> >> > > > >
> >> > > > > By adopting spotless and a particular configuration as our
> coding
> >> > > > standard,
> >> > > > > and by opting in to automatic application of that policy as part
> >> of
> >> > our
> >> > > > > patch submission process, we also agree that a discontinuity in
> >> > history
> >> > > > > (i.e. 'git blame' will be less useful prior to the application
> of
> >> the
> >> > > > > reformatting than afterward) is an acceptable trade off. If this
> >> is
> >> > > true
> >> > > > we
> >> > > > > can move forward. If not it needs more discussion.
> >> > > > >
> >> > > > >
> >> > > > > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <
> >> apurtell@apache.org>
> >> > > > > wrote:
> >> > > > >
> >> > > > > > Yes, I figure we should run spotless on all live branch-2x
> >> branches
> >> > > to
> >> > > > > get
> >> > > > > > a baseline for future contributions and backports.
> >> > > > > >
> >> > > > > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <
> >> > palomino219@gmail.com
> >> > > >
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > >> I’m natural on this. We could remove the ratchetFrom config
> and
> >> > do a
> >> > > > > full
> >> > > > > >> reformat on all branches. The problem is git blame will be
> >> > useless…
> >> > > > > >>
> >> > > > > >> But anyway, if the file has been touched later, git blame
> will
> >> be
> >> > > > broken
> >> > > > > >> too.
> >> > > > > >>
> >> > > > > >> I saw Andrew has already opened a PR. Let’s get it done.
> >> > > > > >>
> >> > > > > >> Thanks.
> >> > > > > >>
> >> > > > > >> Bryan Beaudreault <bbeaudreault@hubspot.com
> >> .invalid>于2022年3月29日
> >> > > > > >> 周二00:55写道：
> >> > > > > >>
> >> > > > > >> > Thanks for your work here Duo!
> >> > > > > >> >
> >> > > > > >> > What should be our convention on committing unrelated
> >> formatting
> >> > > > > changes
> >> > > > > >> > due to spotless? For example, I have a PR
> >> > > > > >> > https://github.com/apache/hbase/pull/4180 open. I rebased
> >> it on
> >> > > > > latest
> >> > > > > >> > master to pull in spotless and ran `mvn spotless:apply` as
> >> > > > suggested.
> >> > > > > As
> >> > > > > >> > you said in the jira, it properly only applied to the
> files I
> >> > > > touched.
> >> > > > > >> But
> >> > > > > >> > some of those files I touched had lots of formatting
> changes
> >> > > > unrelated
> >> > > > > >> to
> >> > > > > >> > my changes. I imagine this will make it harder to review.
> >> > > > > >> >
> >> > > > > >> > I wonder if we should do a single pass of spotbugs:apply to
> >> the
> >> > > main
> >> > > > > >> active
> >> > > > > >> > branches to avoid cases like this. It would probably be a
> >> big PR
> >> > > but
> >> > > > > >> should
> >> > > > > >> > be relatively safe due to just being formatting. Maybe this
> >> was
> >> > > > > already
> >> > > > > >> in
> >> > > > > >> > your plans.
> >> > > > > >> >
> >> > > > > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
> >> > > > palomino219@gmail.com>
> >> > > > > >> > wrote:
> >> > > > > >> >
> >> > > > > >> > > Thanks all for chimming in. HBASE-26617 has been resolved
> >> and
> >> > > the
> >> > > > > >> patch
> >> > > > > >> > has
> >> > > > > >> > > been committed to branch-2.5+.
> >> > > > > >> > >
> >> > > > > >> > > Will start to work on HBASE-26892 to integrate spotless
> >> check
> >> > in
> >> > > > our
> >> > > > > >> pre
> >> > > > > >> > > commit build.
> >> > > > > >> > >
> >> > > > > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
> >> > > > 于2022年3月26日周六
> >> > > > > >> > > 02:35写道：
> >> > > > > >> > >
> >> > > > > >> > > > +1, agreed. It would be nice to backport to branch-2 as
> >> well
> >> > > so
> >> > > > we
> >> > > > > >> can
> >> > > > > >> > > keep
> >> > > > > >> > > > the branches somewhat consistent in style at least.
> >> > > > > >> > > >
> >> > > > > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> >> > > > > >> huaxiangsun@apache.org>
> >> > > > > >> > > > wrote:
> >> > > > > >> > > >
> >> > > > > >> > > > > +1, a very nice and helpful feature.
> >> > > > > >> > > > >
> >> > > > > >> > > > >
> >> > > > > >> > > > >
> >> > > > > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> >> > > > > >> > > > > > Revive.
> >> > > > > >> > > > > >
> >> > > > > >> > > > > > Will wait for another couple of days, if no big
> >> > > objections,
> >> > > > I
> >> > > > > >> will
> >> > > > > >> > > move
> >> > > > > >> > > > > > forward to integrate spotless into our active
> >> branches.
> >> > > > > >> > > > > >
> >> > > > > >> > > > > > Thanks.
> >> > > > > >> > > > > >
> >> > > > > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com>
> 于2022年3月15日周二
> >> > > > 21:17写道：
> >> > > > > >> > > > > >
> >> > > > > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> >> > > > > implemented
> >> > > > > >> a
> >> > > > > >> > PR
> >> > > > > >> > > > for
> >> > > > > >> > > > > > > master branch.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > https://github.com/apache/hbase/pull/4214
> >> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
> >> > > > > >> > > > > <https://github.com/apache/hbase/pull/4214
> >> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
> >> > > > > >> > > >
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > The PR is a bit large because it will also format
> >> the
> >> > > pom,
> >> > > > > the
> >> > > > > >> > > actual
> >> > > > > >> > > > > > > changes are here
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > >
> >> > > > > >> > > > >
> >> > > > > >> > > >
> >> > > > > >> > >
> >> > > > > >> >
> >> > > > > >>
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > > > >> > > <
> >> > > > > >> >
> >> > > > > >>
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > > > >> > >
> >> > > > > >> > > > > <
> >> > > > > >> > > >
> >> > > > > >> > >
> >> > > > > >> >
> >> > > > > >>
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > > > >> > > <
> >> > > > > >> >
> >> > > > > >>
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > > > >> > >
> >> > > > > >> > > > >
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > In the PR I make use of the eclipse formatter and
> >> > import
> >> > > > > order
> >> > > > > >> > file
> >> > > > > >> > > > to
> >> > > > > >> > > > > > > format our java file. For pom, I just use most of
> >> the
> >> > > > > default
> >> > > > > >> > > > > formatter,
> >> > > > > >> > > > > > > the only exception is to expand empty element.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > The spotless plugin support setting a ratchetFrom
> >> > > option,
> >> > > > > >> which
> >> > > > > >> > > could
> >> > > > > >> > > > > be a
> >> > > > > >> > > > > > > commit or a tag. Only files changed after this
> >> commit
> >> > > will
> >> > > > > be
> >> > > > > >> > > > > formatted, so
> >> > > > > >> > > > > > > we can avoid generating a very big patch when
> >> running
> >> > > > > >> > > spotless:apply.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > So after we get this in, before submitting a PR,
> >> you
> >> > can
> >> > > > > just
> >> > > > > >> > type
> >> > > > > >> > > > mvn
> >> > > > > >> > > > > > > spotless:apply, then most of the checkstyle
> issues
> >> > will
> >> > > be
> >> > > > > >> solved
> >> > > > > >> > > > > > > automatically. And we could also add a
> >> spotless:check
> >> > > step
> >> > > > > in
> >> > > > > >> our
> >> > > > > >> > > pre
> >> > > > > >> > > > > > > commit job, to make sure we run spotless:apply
> >> before
> >> > > > > >> submitting
> >> > > > > >> > a
> >> > > > > >> > > > PR.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > Just tell me what you think about the above plan.
> >> > > > > Suggestions
> >> > > > > >> are
> >> > > > > >> > > > > always
> >> > > > > >> > > > > > > welcomed.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > > > Thanks.
> >> > > > > >> > > > > > >
> >> > > > > >> > > > > >
> >> > > > > >> > > > >
> >> > > > > >> > > >
> >> > > > > >> > >
> >> > > > > >> >
> >> > > > > >>
> >> > > > > >
> >> > > > > >
> >> > > > > > --
> >> > > > > > Best regards,
> >> > > > > > Andrew
> >> > > > > >
> >> > > > > > Unrest, ignorance distilled, nihilistic imbeciles -
> >> > > > > > It's what we’ve earned
> >> > > > > > Welcome, apocalypse, what’s taken you so long?
> >> > > > > > Bring us the fitting end that we’ve been counting on
> >> > > > > > - A23, Welcome, Apocalypse
> >> > > > > >
> >> > > > >
> >> > > > >
> >> > > > > --
> >> > > > > Best regards,
> >> > > > > Andrew
> >> > > > >
> >> > > > > Unrest, ignorance distilled, nihilistic imbeciles -
> >> > > > > It's what we’ve earned
> >> > > > > Welcome, apocalypse, what’s taken you so long?
> >> > > > > Bring us the fitting end that we’ve been counting on
> >> > > > > - A23, Welcome, Apocalypse
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >>
> >> --
> >> Best regards,
> >> Andrew
> >>
> >> Unrest, ignorance distilled, nihilistic imbeciles -
> >> It's what we’ve earned
> >> Welcome, apocalypse, what’s taken you so long?
> >> Bring us the fitting end that we’ve been counting on
> >> - A23, Welcome, Apocalypse
> >>
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
Just a reminder that some of us have almost reached a consensus on the
final formatter config file.
PTAL about the discussion and also the final style in this PR
https://github.com/apache/hbase/pull/4312
We have also talked about whether we should just use some well known styles
such as google java format.
So please reply here if you have any thoughts on this, i.e, which style do
you prefer more, the current eclipse formatter, or google java format, or
some other styles.
Thanks a lot!
张铎(Duo Zhang) <pa...@gmail.com> 于2022年4月13日周三 07:48写道：
> Some heads up, the current state on the big reformat can be reviewed here
>
> https://github.com/apache/hbase/pull/4312
>
> Shout if you have any questions about the formatter, we could try to tweak
> the eclipse formatter to see if it could work as expected.
>
> Thanks.
>
> Andrew Purtell <ap...@apache.org> 于2022年4月3日周日 02:34写道：
>
>> I think we have consensus to move forward with application of the PRs, so
>> let's fine tune the formatting configuration, and then regenerate and
>> apply
>> them.
>>
>>
>> On Thu, Mar 31, 2022 at 11:03 PM 张铎(Duo Zhang) <pa...@gmail.com>
>> wrote:
>>
>> > FWIW, you could do a 'git reset --hard' to the commit before the big
>> > reformat and do a 'git blame' again on command line.
>> >
>> > Nick Dimiduk <nd...@apache.org> 于2022年4月1日周五 13:58写道：
>> >
>> > > I noticed that GitHub’s blame UI has a “show before this commit”
>> feature,
>> > > which allowed me to look at the blame info before the reformat was
>> > applied.
>> > > I assume the same is possible on the command line.
>> > >
>> > > On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com>
>> > wrote:
>> > >
>> > > > +1
>> > > >
>> > > > Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
>> > > >
>> > > > > Now that I understand what 'ratchetFrom' in the plugin
>> configuration
>> > > was
>> > > > > doing, I have removed it, and resubmitted the PRs. They are all
>> > > > ridiculous
>> > > > > and expected. Just about every file in the source is touched. On
>> > master
>> > > > > branch 4679 files are modified out of 5507 (excluding .git/).
>> There
>> > are
>> > > > > similar results for other branches.
>> > > > >
>> > > > > I think we should first spot check the PRs to determine if
>> spotless
>> > > > > configuration should be fine tuned. Then once it is acceptable the
>> > PRs
>> > > > can
>> > > > > be updated with a re-application. Merging them applies a one shot
>> > > > > reformatting to all live branches. This is desirable so that going
>> > > > forward
>> > > > > it will be easier for reviewers and committers to manage PRs and
>> > > commits
>> > > > to
>> > > > > multiple branches. Put another way, if we don't do it, merge
>> > conflicts
>> > > > are
>> > > > > more likely.
>> > > > >
>> > > > > By adopting spotless and a particular configuration as our coding
>> > > > standard,
>> > > > > and by opting in to automatic application of that policy as part
>> of
>> > our
>> > > > > patch submission process, we also agree that a discontinuity in
>> > history
>> > > > > (i.e. 'git blame' will be less useful prior to the application of
>> the
>> > > > > reformatting than afterward) is an acceptable trade off. If this
>> is
>> > > true
>> > > > we
>> > > > > can move forward. If not it needs more discussion.
>> > > > >
>> > > > >
>> > > > > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <
>> apurtell@apache.org>
>> > > > > wrote:
>> > > > >
>> > > > > > Yes, I figure we should run spotless on all live branch-2x
>> branches
>> > > to
>> > > > > get
>> > > > > > a baseline for future contributions and backports.
>> > > > > >
>> > > > > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <
>> > palomino219@gmail.com
>> > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > >> I’m natural on this. We could remove the ratchetFrom config and
>> > do a
>> > > > > full
>> > > > > >> reformat on all branches. The problem is git blame will be
>> > useless…
>> > > > > >>
>> > > > > >> But anyway, if the file has been touched later, git blame will
>> be
>> > > > broken
>> > > > > >> too.
>> > > > > >>
>> > > > > >> I saw Andrew has already opened a PR. Let’s get it done.
>> > > > > >>
>> > > > > >> Thanks.
>> > > > > >>
>> > > > > >> Bryan Beaudreault <bbeaudreault@hubspot.com
>> .invalid>于2022年3月29日
>> > > > > >> 周二00:55写道：
>> > > > > >>
>> > > > > >> > Thanks for your work here Duo!
>> > > > > >> >
>> > > > > >> > What should be our convention on committing unrelated
>> formatting
>> > > > > changes
>> > > > > >> > due to spotless? For example, I have a PR
>> > > > > >> > https://github.com/apache/hbase/pull/4180 open. I rebased
>> it on
>> > > > > latest
>> > > > > >> > master to pull in spotless and ran `mvn spotless:apply` as
>> > > > suggested.
>> > > > > As
>> > > > > >> > you said in the jira, it properly only applied to the files I
>> > > > touched.
>> > > > > >> But
>> > > > > >> > some of those files I touched had lots of formatting changes
>> > > > unrelated
>> > > > > >> to
>> > > > > >> > my changes. I imagine this will make it harder to review.
>> > > > > >> >
>> > > > > >> > I wonder if we should do a single pass of spotbugs:apply to
>> the
>> > > main
>> > > > > >> active
>> > > > > >> > branches to avoid cases like this. It would probably be a
>> big PR
>> > > but
>> > > > > >> should
>> > > > > >> > be relatively safe due to just being formatting. Maybe this
>> was
>> > > > > already
>> > > > > >> in
>> > > > > >> > your plans.
>> > > > > >> >
>> > > > > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
>> > > > palomino219@gmail.com>
>> > > > > >> > wrote:
>> > > > > >> >
>> > > > > >> > > Thanks all for chimming in. HBASE-26617 has been resolved
>> and
>> > > the
>> > > > > >> patch
>> > > > > >> > has
>> > > > > >> > > been committed to branch-2.5+.
>> > > > > >> > >
>> > > > > >> > > Will start to work on HBASE-26892 to integrate spotless
>> check
>> > in
>> > > > our
>> > > > > >> pre
>> > > > > >> > > commit build.
>> > > > > >> > >
>> > > > > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
>> > > > 于2022年3月26日周六
>> > > > > >> > > 02:35写道：
>> > > > > >> > >
>> > > > > >> > > > +1, agreed. It would be nice to backport to branch-2 as
>> well
>> > > so
>> > > > we
>> > > > > >> can
>> > > > > >> > > keep
>> > > > > >> > > > the branches somewhat consistent in style at least.
>> > > > > >> > > >
>> > > > > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
>> > > > > >> huaxiangsun@apache.org>
>> > > > > >> > > > wrote:
>> > > > > >> > > >
>> > > > > >> > > > > +1, a very nice and helpful feature.
>> > > > > >> > > > >
>> > > > > >> > > > >
>> > > > > >> > > > >
>> > > > > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
>> > > > > >> > > > > > Revive.
>> > > > > >> > > > > >
>> > > > > >> > > > > > Will wait for another couple of days, if no big
>> > > objections,
>> > > > I
>> > > > > >> will
>> > > > > >> > > move
>> > > > > >> > > > > > forward to integrate spotless into our active
>> branches.
>> > > > > >> > > > > >
>> > > > > >> > > > > > Thanks.
>> > > > > >> > > > > >
>> > > > > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二
>> > > > 21:17写道：
>> > > > > >> > > > > >
>> > > > > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
>> > > > > implemented
>> > > > > >> a
>> > > > > >> > PR
>> > > > > >> > > > for
>> > > > > >> > > > > > > master branch.
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > https://github.com/apache/hbase/pull/4214
>> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
>> > > > > >> > > > > <https://github.com/apache/hbase/pull/4214
>> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
>> > > > > >> > > >
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > The PR is a bit large because it will also format
>> the
>> > > pom,
>> > > > > the
>> > > > > >> > > actual
>> > > > > >> > > > > > > changes are here
>> > > > > >> > > > > > >
>> > > > > >> > > > > > >
>> > > > > >> > > > > > >
>> > > > > >> > > > >
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> >
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > > > >> > > <
>> > > > > >> >
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > > > >> > >
>> > > > > >> > > > > <
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> >
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > > > >> > > <
>> > > > > >> >
>> > > > > >>
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > > > >> > >
>> > > > > >> > > > >
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > In the PR I make use of the eclipse formatter and
>> > import
>> > > > > order
>> > > > > >> > file
>> > > > > >> > > > to
>> > > > > >> > > > > > > format our java file. For pom, I just use most of
>> the
>> > > > > default
>> > > > > >> > > > > formatter,
>> > > > > >> > > > > > > the only exception is to expand empty element.
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > The spotless plugin support setting a ratchetFrom
>> > > option,
>> > > > > >> which
>> > > > > >> > > could
>> > > > > >> > > > > be a
>> > > > > >> > > > > > > commit or a tag. Only files changed after this
>> commit
>> > > will
>> > > > > be
>> > > > > >> > > > > formatted, so
>> > > > > >> > > > > > > we can avoid generating a very big patch when
>> running
>> > > > > >> > > spotless:apply.
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > So after we get this in, before submitting a PR,
>> you
>> > can
>> > > > > just
>> > > > > >> > type
>> > > > > >> > > > mvn
>> > > > > >> > > > > > > spotless:apply, then most of the checkstyle issues
>> > will
>> > > be
>> > > > > >> solved
>> > > > > >> > > > > > > automatically. And we could also add a
>> spotless:check
>> > > step
>> > > > > in
>> > > > > >> our
>> > > > > >> > > pre
>> > > > > >> > > > > > > commit job, to make sure we run spotless:apply
>> before
>> > > > > >> submitting
>> > > > > >> > a
>> > > > > >> > > > PR.
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > Just tell me what you think about the above plan.
>> > > > > Suggestions
>> > > > > >> are
>> > > > > >> > > > > always
>> > > > > >> > > > > > > welcomed.
>> > > > > >> > > > > > >
>> > > > > >> > > > > > > Thanks.
>> > > > > >> > > > > > >
>> > > > > >> > > > > >
>> > > > > >> > > > >
>> > > > > >> > > >
>> > > > > >> > >
>> > > > > >> >
>> > > > > >>
>> > > > > >
>> > > > > >
>> > > > > > --
>> > > > > > Best regards,
>> > > > > > Andrew
>> > > > > >
>> > > > > > Unrest, ignorance distilled, nihilistic imbeciles -
>> > > > > > It's what we’ve earned
>> > > > > > Welcome, apocalypse, what’s taken you so long?
>> > > > > > Bring us the fitting end that we’ve been counting on
>> > > > > > - A23, Welcome, Apocalypse
>> > > > > >
>> > > > >
>> > > > >
>> > > > > --
>> > > > > Best regards,
>> > > > > Andrew
>> > > > >
>> > > > > Unrest, ignorance distilled, nihilistic imbeciles -
>> > > > > It's what we’ve earned
>> > > > > Welcome, apocalypse, what’s taken you so long?
>> > > > > Bring us the fitting end that we’ve been counting on
>> > > > > - A23, Welcome, Apocalypse
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>> --
>> Best regards,
>> Andrew
>>
>> Unrest, ignorance distilled, nihilistic imbeciles -
>> It's what we’ve earned
>> Welcome, apocalypse, what’s taken you so long?
>> Bring us the fitting end that we’ve been counting on
>> - A23, Welcome, Apocalypse
>>
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
Some heads up, the current state on the big reformat can be reviewed here
https://github.com/apache/hbase/pull/4312
Shout if you have any questions about the formatter, we could try to tweak
the eclipse formatter to see if it could work as expected.
Thanks.
Andrew Purtell <ap...@apache.org> 于2022年4月3日周日 02:34写道：
> I think we have consensus to move forward with application of the PRs, so
> let's fine tune the formatting configuration, and then regenerate and apply
> them.
>
>
> On Thu, Mar 31, 2022 at 11:03 PM 张铎(Duo Zhang) <pa...@gmail.com>
> wrote:
>
> > FWIW, you could do a 'git reset --hard' to the commit before the big
> > reformat and do a 'git blame' again on command line.
> >
> > Nick Dimiduk <nd...@apache.org> 于2022年4月1日周五 13:58写道：
> >
> > > I noticed that GitHub’s blame UI has a “show before this commit”
> feature,
> > > which allowed me to look at the blame info before the reformat was
> > applied.
> > > I assume the same is possible on the command line.
> > >
> > > On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com>
> > wrote:
> > >
> > > > +1
> > > >
> > > > Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
> > > >
> > > > > Now that I understand what 'ratchetFrom' in the plugin
> configuration
> > > was
> > > > > doing, I have removed it, and resubmitted the PRs. They are all
> > > > ridiculous
> > > > > and expected. Just about every file in the source is touched. On
> > master
> > > > > branch 4679 files are modified out of 5507 (excluding .git/). There
> > are
> > > > > similar results for other branches.
> > > > >
> > > > > I think we should first spot check the PRs to determine if spotless
> > > > > configuration should be fine tuned. Then once it is acceptable the
> > PRs
> > > > can
> > > > > be updated with a re-application. Merging them applies a one shot
> > > > > reformatting to all live branches. This is desirable so that going
> > > > forward
> > > > > it will be easier for reviewers and committers to manage PRs and
> > > commits
> > > > to
> > > > > multiple branches. Put another way, if we don't do it, merge
> > conflicts
> > > > are
> > > > > more likely.
> > > > >
> > > > > By adopting spotless and a particular configuration as our coding
> > > > standard,
> > > > > and by opting in to automatic application of that policy as part of
> > our
> > > > > patch submission process, we also agree that a discontinuity in
> > history
> > > > > (i.e. 'git blame' will be less useful prior to the application of
> the
> > > > > reformatting than afterward) is an acceptable trade off. If this is
> > > true
> > > > we
> > > > > can move forward. If not it needs more discussion.
> > > > >
> > > > >
> > > > > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <
> apurtell@apache.org>
> > > > > wrote:
> > > > >
> > > > > > Yes, I figure we should run spotless on all live branch-2x
> branches
> > > to
> > > > > get
> > > > > > a baseline for future contributions and backports.
> > > > > >
> > > > > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <
> > palomino219@gmail.com
> > > >
> > > > > > wrote:
> > > > > >
> > > > > >> I’m natural on this. We could remove the ratchetFrom config and
> > do a
> > > > > full
> > > > > >> reformat on all branches. The problem is git blame will be
> > useless…
> > > > > >>
> > > > > >> But anyway, if the file has been touched later, git blame will
> be
> > > > broken
> > > > > >> too.
> > > > > >>
> > > > > >> I saw Andrew has already opened a PR. Let’s get it done.
> > > > > >>
> > > > > >> Thanks.
> > > > > >>
> > > > > >> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
> > > > > >> 周二00:55写道：
> > > > > >>
> > > > > >> > Thanks for your work here Duo!
> > > > > >> >
> > > > > >> > What should be our convention on committing unrelated
> formatting
> > > > > changes
> > > > > >> > due to spotless? For example, I have a PR
> > > > > >> > https://github.com/apache/hbase/pull/4180 open. I rebased it
> on
> > > > > latest
> > > > > >> > master to pull in spotless and ran `mvn spotless:apply` as
> > > > suggested.
> > > > > As
> > > > > >> > you said in the jira, it properly only applied to the files I
> > > > touched.
> > > > > >> But
> > > > > >> > some of those files I touched had lots of formatting changes
> > > > unrelated
> > > > > >> to
> > > > > >> > my changes. I imagine this will make it harder to review.
> > > > > >> >
> > > > > >> > I wonder if we should do a single pass of spotbugs:apply to
> the
> > > main
> > > > > >> active
> > > > > >> > branches to avoid cases like this. It would probably be a big
> PR
> > > but
> > > > > >> should
> > > > > >> > be relatively safe due to just being formatting. Maybe this
> was
> > > > > already
> > > > > >> in
> > > > > >> > your plans.
> > > > > >> >
> > > > > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
> > > > palomino219@gmail.com>
> > > > > >> > wrote:
> > > > > >> >
> > > > > >> > > Thanks all for chimming in. HBASE-26617 has been resolved
> and
> > > the
> > > > > >> patch
> > > > > >> > has
> > > > > >> > > been committed to branch-2.5+.
> > > > > >> > >
> > > > > >> > > Will start to work on HBASE-26892 to integrate spotless
> check
> > in
> > > > our
> > > > > >> pre
> > > > > >> > > commit build.
> > > > > >> > >
> > > > > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
> > > > 于2022年3月26日周六
> > > > > >> > > 02:35写道：
> > > > > >> > >
> > > > > >> > > > +1, agreed. It would be nice to backport to branch-2 as
> well
> > > so
> > > > we
> > > > > >> can
> > > > > >> > > keep
> > > > > >> > > > the branches somewhat consistent in style at least.
> > > > > >> > > >
> > > > > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> > > > > >> huaxiangsun@apache.org>
> > > > > >> > > > wrote:
> > > > > >> > > >
> > > > > >> > > > > +1, a very nice and helpful feature.
> > > > > >> > > > >
> > > > > >> > > > >
> > > > > >> > > > >
> > > > > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > > > >> > > > > > Revive.
> > > > > >> > > > > >
> > > > > >> > > > > > Will wait for another couple of days, if no big
> > > objections,
> > > > I
> > > > > >> will
> > > > > >> > > move
> > > > > >> > > > > > forward to integrate spotless into our active
> branches.
> > > > > >> > > > > >
> > > > > >> > > > > > Thanks.
> > > > > >> > > > > >
> > > > > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二
> > > > 21:17写道：
> > > > > >> > > > > >
> > > > > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> > > > > implemented
> > > > > >> a
> > > > > >> > PR
> > > > > >> > > > for
> > > > > >> > > > > > > master branch.
> > > > > >> > > > > > >
> > > > > >> > > > > > > https://github.com/apache/hbase/pull/4214
> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > > > >> > > > > <https://github.com/apache/hbase/pull/4214
> > > > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > > > >> > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > The PR is a bit large because it will also format
> the
> > > pom,
> > > > > the
> > > > > >> > > actual
> > > > > >> > > > > > > changes are here
> > > > > >> > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >> > > <
> > > > > >> >
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >> > >
> > > > > >> > > > > <
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >> > > <
> > > > > >> >
> > > > > >>
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >> > >
> > > > > >> > > > >
> > > > > >> > > > > > >
> > > > > >> > > > > > > In the PR I make use of the eclipse formatter and
> > import
> > > > > order
> > > > > >> > file
> > > > > >> > > > to
> > > > > >> > > > > > > format our java file. For pom, I just use most of
> the
> > > > > default
> > > > > >> > > > > formatter,
> > > > > >> > > > > > > the only exception is to expand empty element.
> > > > > >> > > > > > >
> > > > > >> > > > > > > The spotless plugin support setting a ratchetFrom
> > > option,
> > > > > >> which
> > > > > >> > > could
> > > > > >> > > > > be a
> > > > > >> > > > > > > commit or a tag. Only files changed after this
> commit
> > > will
> > > > > be
> > > > > >> > > > > formatted, so
> > > > > >> > > > > > > we can avoid generating a very big patch when
> running
> > > > > >> > > spotless:apply.
> > > > > >> > > > > > >
> > > > > >> > > > > > > So after we get this in, before submitting a PR, you
> > can
> > > > > just
> > > > > >> > type
> > > > > >> > > > mvn
> > > > > >> > > > > > > spotless:apply, then most of the checkstyle issues
> > will
> > > be
> > > > > >> solved
> > > > > >> > > > > > > automatically. And we could also add a
> spotless:check
> > > step
> > > > > in
> > > > > >> our
> > > > > >> > > pre
> > > > > >> > > > > > > commit job, to make sure we run spotless:apply
> before
> > > > > >> submitting
> > > > > >> > a
> > > > > >> > > > PR.
> > > > > >> > > > > > >
> > > > > >> > > > > > > Just tell me what you think about the above plan.
> > > > > Suggestions
> > > > > >> are
> > > > > >> > > > > always
> > > > > >> > > > > > > welcomed.
> > > > > >> > > > > > >
> > > > > >> > > > > > > Thanks.
> > > > > >> > > > > > >
> > > > > >> > > > > >
> > > > > >> > > > >
> > > > > >> > > >
> > > > > >> > >
> > > > > >> >
> > > > > >>
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best regards,
> > > > > > Andrew
> > > > > >
> > > > > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > > > > It's what we’ve earned
> > > > > > Welcome, apocalypse, what’s taken you so long?
> > > > > > Bring us the fitting end that we’ve been counting on
> > > > > > - A23, Welcome, Apocalypse
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Andrew
> > > > >
> > > > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > > > It's what we’ve earned
> > > > > Welcome, apocalypse, what’s taken you so long?
> > > > > Bring us the fitting end that we’ve been counting on
> > > > > - A23, Welcome, Apocalypse
> > > > >
> > > >
> > >
> >
>
>
> --
> Best regards,
> Andrew
>
> Unrest, ignorance distilled, nihilistic imbeciles -
> It's what we’ve earned
> Welcome, apocalypse, what’s taken you so long?
> Bring us the fitting end that we’ve been counting on
> - A23, Welcome, Apocalypse
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Andrew Purtell <ap...@apache.org>.
I think we have consensus to move forward with application of the PRs, so
let's fine tune the formatting configuration, and then regenerate and apply
them.
On Thu, Mar 31, 2022 at 11:03 PM 张铎(Duo Zhang) <pa...@gmail.com>
wrote:
> FWIW, you could do a 'git reset --hard' to the commit before the big
> reformat and do a 'git blame' again on command line.
>
> Nick Dimiduk <nd...@apache.org> 于2022年4月1日周五 13:58写道：
>
> > I noticed that GitHub’s blame UI has a “show before this commit” feature,
> > which allowed me to look at the blame info before the reformat was
> applied.
> > I assume the same is possible on the command line.
> >
> > On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com>
> wrote:
> >
> > > +1
> > >
> > > Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
> > >
> > > > Now that I understand what 'ratchetFrom' in the plugin configuration
> > was
> > > > doing, I have removed it, and resubmitted the PRs. They are all
> > > ridiculous
> > > > and expected. Just about every file in the source is touched. On
> master
> > > > branch 4679 files are modified out of 5507 (excluding .git/). There
> are
> > > > similar results for other branches.
> > > >
> > > > I think we should first spot check the PRs to determine if spotless
> > > > configuration should be fine tuned. Then once it is acceptable the
> PRs
> > > can
> > > > be updated with a re-application. Merging them applies a one shot
> > > > reformatting to all live branches. This is desirable so that going
> > > forward
> > > > it will be easier for reviewers and committers to manage PRs and
> > commits
> > > to
> > > > multiple branches. Put another way, if we don't do it, merge
> conflicts
> > > are
> > > > more likely.
> > > >
> > > > By adopting spotless and a particular configuration as our coding
> > > standard,
> > > > and by opting in to automatic application of that policy as part of
> our
> > > > patch submission process, we also agree that a discontinuity in
> history
> > > > (i.e. 'git blame' will be less useful prior to the application of the
> > > > reformatting than afterward) is an acceptable trade off. If this is
> > true
> > > we
> > > > can move forward. If not it needs more discussion.
> > > >
> > > >
> > > > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <ap...@apache.org>
> > > > wrote:
> > > >
> > > > > Yes, I figure we should run spotless on all live branch-2x branches
> > to
> > > > get
> > > > > a baseline for future contributions and backports.
> > > > >
> > > > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <
> palomino219@gmail.com
> > >
> > > > > wrote:
> > > > >
> > > > >> I’m natural on this. We could remove the ratchetFrom config and
> do a
> > > > full
> > > > >> reformat on all branches. The problem is git blame will be
> useless…
> > > > >>
> > > > >> But anyway, if the file has been touched later, git blame will be
> > > broken
> > > > >> too.
> > > > >>
> > > > >> I saw Andrew has already opened a PR. Let’s get it done.
> > > > >>
> > > > >> Thanks.
> > > > >>
> > > > >> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
> > > > >> 周二00:55写道：
> > > > >>
> > > > >> > Thanks for your work here Duo!
> > > > >> >
> > > > >> > What should be our convention on committing unrelated formatting
> > > > changes
> > > > >> > due to spotless? For example, I have a PR
> > > > >> > https://github.com/apache/hbase/pull/4180 open. I rebased it on
> > > > latest
> > > > >> > master to pull in spotless and ran `mvn spotless:apply` as
> > > suggested.
> > > > As
> > > > >> > you said in the jira, it properly only applied to the files I
> > > touched.
> > > > >> But
> > > > >> > some of those files I touched had lots of formatting changes
> > > unrelated
> > > > >> to
> > > > >> > my changes. I imagine this will make it harder to review.
> > > > >> >
> > > > >> > I wonder if we should do a single pass of spotbugs:apply to the
> > main
> > > > >> active
> > > > >> > branches to avoid cases like this. It would probably be a big PR
> > but
> > > > >> should
> > > > >> > be relatively safe due to just being formatting. Maybe this was
> > > > already
> > > > >> in
> > > > >> > your plans.
> > > > >> >
> > > > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
> > > palomino219@gmail.com>
> > > > >> > wrote:
> > > > >> >
> > > > >> > > Thanks all for chimming in. HBASE-26617 has been resolved and
> > the
> > > > >> patch
> > > > >> > has
> > > > >> > > been committed to branch-2.5+.
> > > > >> > >
> > > > >> > > Will start to work on HBASE-26892 to integrate spotless check
> in
> > > our
> > > > >> pre
> > > > >> > > commit build.
> > > > >> > >
> > > > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
> > > 于2022年3月26日周六
> > > > >> > > 02:35写道：
> > > > >> > >
> > > > >> > > > +1, agreed. It would be nice to backport to branch-2 as well
> > so
> > > we
> > > > >> can
> > > > >> > > keep
> > > > >> > > > the branches somewhat consistent in style at least.
> > > > >> > > >
> > > > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> > > > >> huaxiangsun@apache.org>
> > > > >> > > > wrote:
> > > > >> > > >
> > > > >> > > > > +1, a very nice and helpful feature.
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > >
> > > > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > > >> > > > > > Revive.
> > > > >> > > > > >
> > > > >> > > > > > Will wait for another couple of days, if no big
> > objections,
> > > I
> > > > >> will
> > > > >> > > move
> > > > >> > > > > > forward to integrate spotless into our active branches.
> > > > >> > > > > >
> > > > >> > > > > > Thanks.
> > > > >> > > > > >
> > > > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二
> > > 21:17写道：
> > > > >> > > > > >
> > > > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> > > > implemented
> > > > >> a
> > > > >> > PR
> > > > >> > > > for
> > > > >> > > > > > > master branch.
> > > > >> > > > > > >
> > > > >> > > > > > > https://github.com/apache/hbase/pull/4214
> > > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > > >> > > > > <https://github.com/apache/hbase/pull/4214
> > > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > > >> > > >
> > > > >> > > > > > >
> > > > >> > > > > > > The PR is a bit large because it will also format the
> > pom,
> > > > the
> > > > >> > > actual
> > > > >> > > > > > > changes are here
> > > > >> > > > > > >
> > > > >> > > > > > >
> > > > >> > > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >> > > <
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >> > >
> > > > >> > > > > <
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >> > > <
> > > > >> >
> > > > >>
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >> > >
> > > > >> > > > >
> > > > >> > > > > > >
> > > > >> > > > > > > In the PR I make use of the eclipse formatter and
> import
> > > > order
> > > > >> > file
> > > > >> > > > to
> > > > >> > > > > > > format our java file. For pom, I just use most of the
> > > > default
> > > > >> > > > > formatter,
> > > > >> > > > > > > the only exception is to expand empty element.
> > > > >> > > > > > >
> > > > >> > > > > > > The spotless plugin support setting a ratchetFrom
> > option,
> > > > >> which
> > > > >> > > could
> > > > >> > > > > be a
> > > > >> > > > > > > commit or a tag. Only files changed after this commit
> > will
> > > > be
> > > > >> > > > > formatted, so
> > > > >> > > > > > > we can avoid generating a very big patch when running
> > > > >> > > spotless:apply.
> > > > >> > > > > > >
> > > > >> > > > > > > So after we get this in, before submitting a PR, you
> can
> > > > just
> > > > >> > type
> > > > >> > > > mvn
> > > > >> > > > > > > spotless:apply, then most of the checkstyle issues
> will
> > be
> > > > >> solved
> > > > >> > > > > > > automatically. And we could also add a spotless:check
> > step
> > > > in
> > > > >> our
> > > > >> > > pre
> > > > >> > > > > > > commit job, to make sure we run spotless:apply before
> > > > >> submitting
> > > > >> > a
> > > > >> > > > PR.
> > > > >> > > > > > >
> > > > >> > > > > > > Just tell me what you think about the above plan.
> > > > Suggestions
> > > > >> are
> > > > >> > > > > always
> > > > >> > > > > > > welcomed.
> > > > >> > > > > > >
> > > > >> > > > > > > Thanks.
> > > > >> > > > > > >
> > > > >> > > > > >
> > > > >> > > > >
> > > > >> > > >
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Andrew
> > > > >
> > > > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > > > It's what we’ve earned
> > > > > Welcome, apocalypse, what’s taken you so long?
> > > > > Bring us the fitting end that we’ve been counting on
> > > > > - A23, Welcome, Apocalypse
> > > > >
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Andrew
> > > >
> > > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > > It's what we’ve earned
> > > > Welcome, apocalypse, what’s taken you so long?
> > > > Bring us the fitting end that we’ve been counting on
> > > > - A23, Welcome, Apocalypse
> > > >
> > >
> >
>
--
Best regards,
Andrew
Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
- A23, Welcome, Apocalypse
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
FWIW, you could do a 'git reset --hard' to the commit before the big
reformat and do a 'git blame' again on command line.
Nick Dimiduk <nd...@apache.org> 于2022年4月1日周五 13:58写道：
> I noticed that GitHub’s blame UI has a “show before this commit” feature,
> which allowed me to look at the blame info before the reformat was applied.
> I assume the same is possible on the command line.
>
> On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> > +1
> >
> > Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
> >
> > > Now that I understand what 'ratchetFrom' in the plugin configuration
> was
> > > doing, I have removed it, and resubmitted the PRs. They are all
> > ridiculous
> > > and expected. Just about every file in the source is touched. On master
> > > branch 4679 files are modified out of 5507 (excluding .git/). There are
> > > similar results for other branches.
> > >
> > > I think we should first spot check the PRs to determine if spotless
> > > configuration should be fine tuned. Then once it is acceptable the PRs
> > can
> > > be updated with a re-application. Merging them applies a one shot
> > > reformatting to all live branches. This is desirable so that going
> > forward
> > > it will be easier for reviewers and committers to manage PRs and
> commits
> > to
> > > multiple branches. Put another way, if we don't do it, merge conflicts
> > are
> > > more likely.
> > >
> > > By adopting spotless and a particular configuration as our coding
> > standard,
> > > and by opting in to automatic application of that policy as part of our
> > > patch submission process, we also agree that a discontinuity in history
> > > (i.e. 'git blame' will be less useful prior to the application of the
> > > reformatting than afterward) is an acceptable trade off. If this is
> true
> > we
> > > can move forward. If not it needs more discussion.
> > >
> > >
> > > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <ap...@apache.org>
> > > wrote:
> > >
> > > > Yes, I figure we should run spotless on all live branch-2x branches
> to
> > > get
> > > > a baseline for future contributions and backports.
> > > >
> > > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <palomino219@gmail.com
> >
> > > > wrote:
> > > >
> > > >> I’m natural on this. We could remove the ratchetFrom config and do a
> > > full
> > > >> reformat on all branches. The problem is git blame will be useless…
> > > >>
> > > >> But anyway, if the file has been touched later, git blame will be
> > broken
> > > >> too.
> > > >>
> > > >> I saw Andrew has already opened a PR. Let’s get it done.
> > > >>
> > > >> Thanks.
> > > >>
> > > >> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
> > > >> 周二00:55写道：
> > > >>
> > > >> > Thanks for your work here Duo!
> > > >> >
> > > >> > What should be our convention on committing unrelated formatting
> > > changes
> > > >> > due to spotless? For example, I have a PR
> > > >> > https://github.com/apache/hbase/pull/4180 open. I rebased it on
> > > latest
> > > >> > master to pull in spotless and ran `mvn spotless:apply` as
> > suggested.
> > > As
> > > >> > you said in the jira, it properly only applied to the files I
> > touched.
> > > >> But
> > > >> > some of those files I touched had lots of formatting changes
> > unrelated
> > > >> to
> > > >> > my changes. I imagine this will make it harder to review.
> > > >> >
> > > >> > I wonder if we should do a single pass of spotbugs:apply to the
> main
> > > >> active
> > > >> > branches to avoid cases like this. It would probably be a big PR
> but
> > > >> should
> > > >> > be relatively safe due to just being formatting. Maybe this was
> > > already
> > > >> in
> > > >> > your plans.
> > > >> >
> > > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
> > palomino219@gmail.com>
> > > >> > wrote:
> > > >> >
> > > >> > > Thanks all for chimming in. HBASE-26617 has been resolved and
> the
> > > >> patch
> > > >> > has
> > > >> > > been committed to branch-2.5+.
> > > >> > >
> > > >> > > Will start to work on HBASE-26892 to integrate spotless check in
> > our
> > > >> pre
> > > >> > > commit build.
> > > >> > >
> > > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
> > 于2022年3月26日周六
> > > >> > > 02:35写道：
> > > >> > >
> > > >> > > > +1, agreed. It would be nice to backport to branch-2 as well
> so
> > we
> > > >> can
> > > >> > > keep
> > > >> > > > the branches somewhat consistent in style at least.
> > > >> > > >
> > > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> > > >> huaxiangsun@apache.org>
> > > >> > > > wrote:
> > > >> > > >
> > > >> > > > > +1, a very nice and helpful feature.
> > > >> > > > >
> > > >> > > > >
> > > >> > > > >
> > > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > >> > > > > > Revive.
> > > >> > > > > >
> > > >> > > > > > Will wait for another couple of days, if no big
> objections,
> > I
> > > >> will
> > > >> > > move
> > > >> > > > > > forward to integrate spotless into our active branches.
> > > >> > > > > >
> > > >> > > > > > Thanks.
> > > >> > > > > >
> > > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二
> > 21:17写道：
> > > >> > > > > >
> > > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> > > implemented
> > > >> a
> > > >> > PR
> > > >> > > > for
> > > >> > > > > > > master branch.
> > > >> > > > > > >
> > > >> > > > > > > https://github.com/apache/hbase/pull/4214
> > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > >> > > > > <https://github.com/apache/hbase/pull/4214
> > > >> > > <https://github.com/apache/hbase/pull/4214>
> > > >> > > >
> > > >> > > > > > >
> > > >> > > > > > > The PR is a bit large because it will also format the
> pom,
> > > the
> > > >> > > actual
> > > >> > > > > > > changes are here
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >> > > <
> > > >> >
> > > >>
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >> > >
> > > >> > > > > <
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >> > > <
> > > >> >
> > > >>
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >> > >
> > > >> > > > >
> > > >> > > > > > >
> > > >> > > > > > > In the PR I make use of the eclipse formatter and import
> > > order
> > > >> > file
> > > >> > > > to
> > > >> > > > > > > format our java file. For pom, I just use most of the
> > > default
> > > >> > > > > formatter,
> > > >> > > > > > > the only exception is to expand empty element.
> > > >> > > > > > >
> > > >> > > > > > > The spotless plugin support setting a ratchetFrom
> option,
> > > >> which
> > > >> > > could
> > > >> > > > > be a
> > > >> > > > > > > commit or a tag. Only files changed after this commit
> will
> > > be
> > > >> > > > > formatted, so
> > > >> > > > > > > we can avoid generating a very big patch when running
> > > >> > > spotless:apply.
> > > >> > > > > > >
> > > >> > > > > > > So after we get this in, before submitting a PR, you can
> > > just
> > > >> > type
> > > >> > > > mvn
> > > >> > > > > > > spotless:apply, then most of the checkstyle issues will
> be
> > > >> solved
> > > >> > > > > > > automatically. And we could also add a spotless:check
> step
> > > in
> > > >> our
> > > >> > > pre
> > > >> > > > > > > commit job, to make sure we run spotless:apply before
> > > >> submitting
> > > >> > a
> > > >> > > > PR.
> > > >> > > > > > >
> > > >> > > > > > > Just tell me what you think about the above plan.
> > > Suggestions
> > > >> are
> > > >> > > > > always
> > > >> > > > > > > welcomed.
> > > >> > > > > > >
> > > >> > > > > > > Thanks.
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > > > --
> > > > Best regards,
> > > > Andrew
> > > >
> > > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > > It's what we’ve earned
> > > > Welcome, apocalypse, what’s taken you so long?
> > > > Bring us the fitting end that we’ve been counting on
> > > > - A23, Welcome, Apocalypse
> > > >
> > >
> > >
> > > --
> > > Best regards,
> > > Andrew
> > >
> > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > It's what we’ve earned
> > > Welcome, apocalypse, what’s taken you so long?
> > > Bring us the fitting end that we’ve been counting on
> > > - A23, Welcome, Apocalypse
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Nick Dimiduk <nd...@apache.org>.
I noticed that GitHub’s blame UI has a “show before this commit” feature,
which allowed me to look at the blame info before the reformat was applied.
I assume the same is possible on the command line.
On Fri, Apr 1, 2022 at 04:06 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> +1
>
> Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
>
> > Now that I understand what 'ratchetFrom' in the plugin configuration was
> > doing, I have removed it, and resubmitted the PRs. They are all
> ridiculous
> > and expected. Just about every file in the source is touched. On master
> > branch 4679 files are modified out of 5507 (excluding .git/). There are
> > similar results for other branches.
> >
> > I think we should first spot check the PRs to determine if spotless
> > configuration should be fine tuned. Then once it is acceptable the PRs
> can
> > be updated with a re-application. Merging them applies a one shot
> > reformatting to all live branches. This is desirable so that going
> forward
> > it will be easier for reviewers and committers to manage PRs and commits
> to
> > multiple branches. Put another way, if we don't do it, merge conflicts
> are
> > more likely.
> >
> > By adopting spotless and a particular configuration as our coding
> standard,
> > and by opting in to automatic application of that policy as part of our
> > patch submission process, we also agree that a discontinuity in history
> > (i.e. 'git blame' will be less useful prior to the application of the
> > reformatting than afterward) is an acceptable trade off. If this is true
> we
> > can move forward. If not it needs more discussion.
> >
> >
> > On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <ap...@apache.org>
> > wrote:
> >
> > > Yes, I figure we should run spotless on all live branch-2x branches to
> > get
> > > a baseline for future contributions and backports.
> > >
> > > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <pa...@gmail.com>
> > > wrote:
> > >
> > >> I’m natural on this. We could remove the ratchetFrom config and do a
> > full
> > >> reformat on all branches. The problem is git blame will be useless…
> > >>
> > >> But anyway, if the file has been touched later, git blame will be
> broken
> > >> too.
> > >>
> > >> I saw Andrew has already opened a PR. Let’s get it done.
> > >>
> > >> Thanks.
> > >>
> > >> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
> > >> 周二00:55写道：
> > >>
> > >> > Thanks for your work here Duo!
> > >> >
> > >> > What should be our convention on committing unrelated formatting
> > changes
> > >> > due to spotless? For example, I have a PR
> > >> > https://github.com/apache/hbase/pull/4180 open. I rebased it on
> > latest
> > >> > master to pull in spotless and ran `mvn spotless:apply` as
> suggested.
> > As
> > >> > you said in the jira, it properly only applied to the files I
> touched.
> > >> But
> > >> > some of those files I touched had lots of formatting changes
> unrelated
> > >> to
> > >> > my changes. I imagine this will make it harder to review.
> > >> >
> > >> > I wonder if we should do a single pass of spotbugs:apply to the main
> > >> active
> > >> > branches to avoid cases like this. It would probably be a big PR but
> > >> should
> > >> > be relatively safe due to just being formatting. Maybe this was
> > already
> > >> in
> > >> > your plans.
> > >> >
> > >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <
> palomino219@gmail.com>
> > >> > wrote:
> > >> >
> > >> > > Thanks all for chimming in. HBASE-26617 has been resolved and the
> > >> patch
> > >> > has
> > >> > > been committed to branch-2.5+.
> > >> > >
> > >> > > Will start to work on HBASE-26892 to integrate spotless check in
> our
> > >> pre
> > >> > > commit build.
> > >> > >
> > >> > > Bryan Beaudreault <bb...@hubspot.com.invalid>
> 于2022年3月26日周六
> > >> > > 02:35写道：
> > >> > >
> > >> > > > +1, agreed. It would be nice to backport to branch-2 as well so
> we
> > >> can
> > >> > > keep
> > >> > > > the branches somewhat consistent in style at least.
> > >> > > >
> > >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> > >> huaxiangsun@apache.org>
> > >> > > > wrote:
> > >> > > >
> > >> > > > > +1, a very nice and helpful feature.
> > >> > > > >
> > >> > > > >
> > >> > > > >
> > >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > >> > > > > > Revive.
> > >> > > > > >
> > >> > > > > > Will wait for another couple of days, if no big objections,
> I
> > >> will
> > >> > > move
> > >> > > > > > forward to integrate spotless into our active branches.
> > >> > > > > >
> > >> > > > > > Thanks.
> > >> > > > > >
> > >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二
> 21:17写道：
> > >> > > > > >
> > >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> > implemented
> > >> a
> > >> > PR
> > >> > > > for
> > >> > > > > > > master branch.
> > >> > > > > > >
> > >> > > > > > > https://github.com/apache/hbase/pull/4214
> > >> > > <https://github.com/apache/hbase/pull/4214>
> > >> > > > > <https://github.com/apache/hbase/pull/4214
> > >> > > <https://github.com/apache/hbase/pull/4214>
> > >> > > >
> > >> > > > > > >
> > >> > > > > > > The PR is a bit large because it will also format the pom,
> > the
> > >> > > actual
> > >> > > > > > > changes are here
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >> > > <
> > >> >
> > >>
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >> > >
> > >> > > > > <
> > >> > > >
> > >> > >
> > >> >
> > >>
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >> > > <
> > >> >
> > >>
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >> > >
> > >> > > > >
> > >> > > > > > >
> > >> > > > > > > In the PR I make use of the eclipse formatter and import
> > order
> > >> > file
> > >> > > > to
> > >> > > > > > > format our java file. For pom, I just use most of the
> > default
> > >> > > > > formatter,
> > >> > > > > > > the only exception is to expand empty element.
> > >> > > > > > >
> > >> > > > > > > The spotless plugin support setting a ratchetFrom option,
> > >> which
> > >> > > could
> > >> > > > > be a
> > >> > > > > > > commit or a tag. Only files changed after this commit will
> > be
> > >> > > > > formatted, so
> > >> > > > > > > we can avoid generating a very big patch when running
> > >> > > spotless:apply.
> > >> > > > > > >
> > >> > > > > > > So after we get this in, before submitting a PR, you can
> > just
> > >> > type
> > >> > > > mvn
> > >> > > > > > > spotless:apply, then most of the checkstyle issues will be
> > >> solved
> > >> > > > > > > automatically. And we could also add a spotless:check step
> > in
> > >> our
> > >> > > pre
> > >> > > > > > > commit job, to make sure we run spotless:apply before
> > >> submitting
> > >> > a
> > >> > > > PR.
> > >> > > > > > >
> > >> > > > > > > Just tell me what you think about the above plan.
> > Suggestions
> > >> are
> > >> > > > > always
> > >> > > > > > > welcomed.
> > >> > > > > > >
> > >> > > > > > > Thanks.
> > >> > > > > > >
> > >> > > > > >
> > >> > > > >
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> > >
> > > --
> > > Best regards,
> > > Andrew
> > >
> > > Unrest, ignorance distilled, nihilistic imbeciles -
> > > It's what we’ve earned
> > > Welcome, apocalypse, what’s taken you so long?
> > > Bring us the fitting end that we’ve been counting on
> > > - A23, Welcome, Apocalypse
> > >
> >
> >
> > --
> > Best regards,
> > Andrew
> >
> > Unrest, ignorance distilled, nihilistic imbeciles -
> > It's what we’ve earned
> > Welcome, apocalypse, what’s taken you so long?
> > Bring us the fitting end that we’ve been counting on
> > - A23, Welcome, Apocalypse
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
+1
Andrew Purtell <ap...@apache.org> 于2022年4月1日周五 06:18写道：
> Now that I understand what 'ratchetFrom' in the plugin configuration was
> doing, I have removed it, and resubmitted the PRs. They are all ridiculous
> and expected. Just about every file in the source is touched. On master
> branch 4679 files are modified out of 5507 (excluding .git/). There are
> similar results for other branches.
>
> I think we should first spot check the PRs to determine if spotless
> configuration should be fine tuned. Then once it is acceptable the PRs can
> be updated with a re-application. Merging them applies a one shot
> reformatting to all live branches. This is desirable so that going forward
> it will be easier for reviewers and committers to manage PRs and commits to
> multiple branches. Put another way, if we don't do it, merge conflicts are
> more likely.
>
> By adopting spotless and a particular configuration as our coding standard,
> and by opting in to automatic application of that policy as part of our
> patch submission process, we also agree that a discontinuity in history
> (i.e. 'git blame' will be less useful prior to the application of the
> reformatting than afterward) is an acceptable trade off. If this is true we
> can move forward. If not it needs more discussion.
>
>
> On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <ap...@apache.org>
> wrote:
>
> > Yes, I figure we should run spotless on all live branch-2x branches to
> get
> > a baseline for future contributions and backports.
> >
> > On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <pa...@gmail.com>
> > wrote:
> >
> >> I’m natural on this. We could remove the ratchetFrom config and do a
> full
> >> reformat on all branches. The problem is git blame will be useless…
> >>
> >> But anyway, if the file has been touched later, git blame will be broken
> >> too.
> >>
> >> I saw Andrew has already opened a PR. Let’s get it done.
> >>
> >> Thanks.
> >>
> >> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
> >> 周二00:55写道：
> >>
> >> > Thanks for your work here Duo!
> >> >
> >> > What should be our convention on committing unrelated formatting
> changes
> >> > due to spotless? For example, I have a PR
> >> > https://github.com/apache/hbase/pull/4180 open. I rebased it on
> latest
> >> > master to pull in spotless and ran `mvn spotless:apply` as suggested.
> As
> >> > you said in the jira, it properly only applied to the files I touched.
> >> But
> >> > some of those files I touched had lots of formatting changes unrelated
> >> to
> >> > my changes. I imagine this will make it harder to review.
> >> >
> >> > I wonder if we should do a single pass of spotbugs:apply to the main
> >> active
> >> > branches to avoid cases like this. It would probably be a big PR but
> >> should
> >> > be relatively safe due to just being formatting. Maybe this was
> already
> >> in
> >> > your plans.
> >> >
> >> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <pa...@gmail.com>
> >> > wrote:
> >> >
> >> > > Thanks all for chimming in. HBASE-26617 has been resolved and the
> >> patch
> >> > has
> >> > > been committed to branch-2.5+.
> >> > >
> >> > > Will start to work on HBASE-26892 to integrate spotless check in our
> >> pre
> >> > > commit build.
> >> > >
> >> > > Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六
> >> > > 02:35写道：
> >> > >
> >> > > > +1, agreed. It would be nice to backport to branch-2 as well so we
> >> can
> >> > > keep
> >> > > > the branches somewhat consistent in style at least.
> >> > > >
> >> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
> >> huaxiangsun@apache.org>
> >> > > > wrote:
> >> > > >
> >> > > > > +1, a very nice and helpful feature.
> >> > > > >
> >> > > > >
> >> > > > >
> >> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> >> > > > > > Revive.
> >> > > > > >
> >> > > > > > Will wait for another couple of days, if no big objections, I
> >> will
> >> > > move
> >> > > > > > forward to integrate spotless into our active branches.
> >> > > > > >
> >> > > > > > Thanks.
> >> > > > > >
> >> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> >> > > > > >
> >> > > > > > > I've filed HBASE-26617 a while ago and recently I
> implemented
> >> a
> >> > PR
> >> > > > for
> >> > > > > > > master branch.
> >> > > > > > >
> >> > > > > > > https://github.com/apache/hbase/pull/4214
> >> > > <https://github.com/apache/hbase/pull/4214>
> >> > > > > <https://github.com/apache/hbase/pull/4214
> >> > > <https://github.com/apache/hbase/pull/4214>
> >> > > >
> >> > > > > > >
> >> > > > > > > The PR is a bit large because it will also format the pom,
> the
> >> > > actual
> >> > > > > > > changes are here
> >> > > > > > >
> >> > > > > > >
> >> > > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > <
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > >
> >> > > > > <
> >> > > >
> >> > >
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > > <
> >> >
> >>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >> > >
> >> > > > >
> >> > > > > > >
> >> > > > > > > In the PR I make use of the eclipse formatter and import
> order
> >> > file
> >> > > > to
> >> > > > > > > format our java file. For pom, I just use most of the
> default
> >> > > > > formatter,
> >> > > > > > > the only exception is to expand empty element.
> >> > > > > > >
> >> > > > > > > The spotless plugin support setting a ratchetFrom option,
> >> which
> >> > > could
> >> > > > > be a
> >> > > > > > > commit or a tag. Only files changed after this commit will
> be
> >> > > > > formatted, so
> >> > > > > > > we can avoid generating a very big patch when running
> >> > > spotless:apply.
> >> > > > > > >
> >> > > > > > > So after we get this in, before submitting a PR, you can
> just
> >> > type
> >> > > > mvn
> >> > > > > > > spotless:apply, then most of the checkstyle issues will be
> >> solved
> >> > > > > > > automatically. And we could also add a spotless:check step
> in
> >> our
> >> > > pre
> >> > > > > > > commit job, to make sure we run spotless:apply before
> >> submitting
> >> > a
> >> > > > PR.
> >> > > > > > >
> >> > > > > > > Just tell me what you think about the above plan.
> Suggestions
> >> are
> >> > > > > always
> >> > > > > > > welcomed.
> >> > > > > > >
> >> > > > > > > Thanks.
> >> > > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >
> >
> > --
> > Best regards,
> > Andrew
> >
> > Unrest, ignorance distilled, nihilistic imbeciles -
> > It's what we’ve earned
> > Welcome, apocalypse, what’s taken you so long?
> > Bring us the fitting end that we’ve been counting on
> > - A23, Welcome, Apocalypse
> >
>
>
> --
> Best regards,
> Andrew
>
> Unrest, ignorance distilled, nihilistic imbeciles -
> It's what we’ve earned
> Welcome, apocalypse, what’s taken you so long?
> Bring us the fitting end that we’ve been counting on
> - A23, Welcome, Apocalypse
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Andrew Purtell <ap...@apache.org>.
Now that I understand what 'ratchetFrom' in the plugin configuration was
doing, I have removed it, and resubmitted the PRs. They are all ridiculous
and expected. Just about every file in the source is touched. On master
branch 4679 files are modified out of 5507 (excluding .git/). There are
similar results for other branches.
I think we should first spot check the PRs to determine if spotless
configuration should be fine tuned. Then once it is acceptable the PRs can
be updated with a re-application. Merging them applies a one shot
reformatting to all live branches. This is desirable so that going forward
it will be easier for reviewers and committers to manage PRs and commits to
multiple branches. Put another way, if we don't do it, merge conflicts are
more likely.
By adopting spotless and a particular configuration as our coding standard,
and by opting in to automatic application of that policy as part of our
patch submission process, we also agree that a discontinuity in history
(i.e. 'git blame' will be less useful prior to the application of the
reformatting than afterward) is an acceptable trade off. If this is true we
can move forward. If not it needs more discussion.
On Mon, Mar 28, 2022 at 7:08 PM Andrew Purtell <ap...@apache.org> wrote:
> Yes, I figure we should run spotless on all live branch-2x branches to get
> a baseline for future contributions and backports.
>
> On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <pa...@gmail.com>
> wrote:
>
>> I’m natural on this. We could remove the ratchetFrom config and do a full
>> reformat on all branches. The problem is git blame will be useless…
>>
>> But anyway, if the file has been touched later, git blame will be broken
>> too.
>>
>> I saw Andrew has already opened a PR. Let’s get it done.
>>
>> Thanks.
>>
>> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日
>> 周二00:55写道：
>>
>> > Thanks for your work here Duo!
>> >
>> > What should be our convention on committing unrelated formatting changes
>> > due to spotless? For example, I have a PR
>> > https://github.com/apache/hbase/pull/4180 open. I rebased it on latest
>> > master to pull in spotless and ran `mvn spotless:apply` as suggested. As
>> > you said in the jira, it properly only applied to the files I touched.
>> But
>> > some of those files I touched had lots of formatting changes unrelated
>> to
>> > my changes. I imagine this will make it harder to review.
>> >
>> > I wonder if we should do a single pass of spotbugs:apply to the main
>> active
>> > branches to avoid cases like this. It would probably be a big PR but
>> should
>> > be relatively safe due to just being formatting. Maybe this was already
>> in
>> > your plans.
>> >
>> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <pa...@gmail.com>
>> > wrote:
>> >
>> > > Thanks all for chimming in. HBASE-26617 has been resolved and the
>> patch
>> > has
>> > > been committed to branch-2.5+.
>> > >
>> > > Will start to work on HBASE-26892 to integrate spotless check in our
>> pre
>> > > commit build.
>> > >
>> > > Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六
>> > > 02:35写道：
>> > >
>> > > > +1, agreed. It would be nice to backport to branch-2 as well so we
>> can
>> > > keep
>> > > > the branches somewhat consistent in style at least.
>> > > >
>> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <
>> huaxiangsun@apache.org>
>> > > > wrote:
>> > > >
>> > > > > +1, a very nice and helpful feature.
>> > > > >
>> > > > >
>> > > > >
>> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
>> > > > > > Revive.
>> > > > > >
>> > > > > > Will wait for another couple of days, if no big objections, I
>> will
>> > > move
>> > > > > > forward to integrate spotless into our active branches.
>> > > > > >
>> > > > > > Thanks.
>> > > > > >
>> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
>> > > > > >
>> > > > > > > I've filed HBASE-26617 a while ago and recently I implemented
>> a
>> > PR
>> > > > for
>> > > > > > > master branch.
>> > > > > > >
>> > > > > > > https://github.com/apache/hbase/pull/4214
>> > > <https://github.com/apache/hbase/pull/4214>
>> > > > > <https://github.com/apache/hbase/pull/4214
>> > > <https://github.com/apache/hbase/pull/4214>
>> > > >
>> > > > > > >
>> > > > > > > The PR is a bit large because it will also format the pom, the
>> > > actual
>> > > > > > > changes are here
>> > > > > > >
>> > > > > > >
>> > > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > <
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > >
>> > > > > <
>> > > >
>> > >
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > > <
>> >
>> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>> > >
>> > > > >
>> > > > > > >
>> > > > > > > In the PR I make use of the eclipse formatter and import order
>> > file
>> > > > to
>> > > > > > > format our java file. For pom, I just use most of the default
>> > > > > formatter,
>> > > > > > > the only exception is to expand empty element.
>> > > > > > >
>> > > > > > > The spotless plugin support setting a ratchetFrom option,
>> which
>> > > could
>> > > > > be a
>> > > > > > > commit or a tag. Only files changed after this commit will be
>> > > > > formatted, so
>> > > > > > > we can avoid generating a very big patch when running
>> > > spotless:apply.
>> > > > > > >
>> > > > > > > So after we get this in, before submitting a PR, you can just
>> > type
>> > > > mvn
>> > > > > > > spotless:apply, then most of the checkstyle issues will be
>> solved
>> > > > > > > automatically. And we could also add a spotless:check step in
>> our
>> > > pre
>> > > > > > > commit job, to make sure we run spotless:apply before
>> submitting
>> > a
>> > > > PR.
>> > > > > > >
>> > > > > > > Just tell me what you think about the above plan. Suggestions
>> are
>> > > > > always
>> > > > > > > welcomed.
>> > > > > > >
>> > > > > > > Thanks.
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>
> --
> Best regards,
> Andrew
>
> Unrest, ignorance distilled, nihilistic imbeciles -
> It's what we’ve earned
> Welcome, apocalypse, what’s taken you so long?
> Bring us the fitting end that we’ve been counting on
> - A23, Welcome, Apocalypse
>
--
Best regards,
Andrew
Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
- A23, Welcome, Apocalypse
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Andrew Purtell <ap...@apache.org>.
Yes, I figure we should run spotless on all live branch-2x branches to get
a baseline for future contributions and backports.
On Mon, Mar 28, 2022 at 6:50 PM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> I’m natural on this. We could remove the ratchetFrom config and do a full
> reformat on all branches. The problem is git blame will be useless…
>
> But anyway, if the file has been touched later, git blame will be broken
> too.
>
> I saw Andrew has already opened a PR. Let’s get it done.
>
> Thanks.
>
> Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日 周二00:55写道：
>
> > Thanks for your work here Duo!
> >
> > What should be our convention on committing unrelated formatting changes
> > due to spotless? For example, I have a PR
> > https://github.com/apache/hbase/pull/4180 open. I rebased it on latest
> > master to pull in spotless and ran `mvn spotless:apply` as suggested. As
> > you said in the jira, it properly only applied to the files I touched.
> But
> > some of those files I touched had lots of formatting changes unrelated to
> > my changes. I imagine this will make it harder to review.
> >
> > I wonder if we should do a single pass of spotbugs:apply to the main
> active
> > branches to avoid cases like this. It would probably be a big PR but
> should
> > be relatively safe due to just being formatting. Maybe this was already
> in
> > your plans.
> >
> > On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <pa...@gmail.com>
> > wrote:
> >
> > > Thanks all for chimming in. HBASE-26617 has been resolved and the patch
> > has
> > > been committed to branch-2.5+.
> > >
> > > Will start to work on HBASE-26892 to integrate spotless check in our
> pre
> > > commit build.
> > >
> > > Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六
> > > 02:35写道：
> > >
> > > > +1, agreed. It would be nice to backport to branch-2 as well so we
> can
> > > keep
> > > > the branches somewhat consistent in style at least.
> > > >
> > > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <huaxiangsun@apache.org
> >
> > > > wrote:
> > > >
> > > > > +1, a very nice and helpful feature.
> > > > >
> > > > >
> > > > >
> > > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > > > > Revive.
> > > > > >
> > > > > > Will wait for another couple of days, if no big objections, I
> will
> > > move
> > > > > > forward to integrate spotless into our active branches.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> > > > > >
> > > > > > > I've filed HBASE-26617 a while ago and recently I implemented a
> > PR
> > > > for
> > > > > > > master branch.
> > > > > > >
> > > > > > > https://github.com/apache/hbase/pull/4214
> > > <https://github.com/apache/hbase/pull/4214>
> > > > > <https://github.com/apache/hbase/pull/4214
> > > <https://github.com/apache/hbase/pull/4214>
> > > >
> > > > > > >
> > > > > > > The PR is a bit large because it will also format the pom, the
> > > actual
> > > > > > > changes are here
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > <
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >
> > > > > <
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > <
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >
> > > > >
> > > > > > >
> > > > > > > In the PR I make use of the eclipse formatter and import order
> > file
> > > > to
> > > > > > > format our java file. For pom, I just use most of the default
> > > > > formatter,
> > > > > > > the only exception is to expand empty element.
> > > > > > >
> > > > > > > The spotless plugin support setting a ratchetFrom option, which
> > > could
> > > > > be a
> > > > > > > commit or a tag. Only files changed after this commit will be
> > > > > formatted, so
> > > > > > > we can avoid generating a very big patch when running
> > > spotless:apply.
> > > > > > >
> > > > > > > So after we get this in, before submitting a PR, you can just
> > type
> > > > mvn
> > > > > > > spotless:apply, then most of the checkstyle issues will be
> solved
> > > > > > > automatically. And we could also add a spotless:check step in
> our
> > > pre
> > > > > > > commit job, to make sure we run spotless:apply before
> submitting
> > a
> > > > PR.
> > > > > > >
> > > > > > > Just tell me what you think about the above plan. Suggestions
> are
> > > > > always
> > > > > > > welcomed.
> > > > > > >
> > > > > > > Thanks.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
Best regards,
Andrew
Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
- A23, Welcome, Apocalypse
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
I’m natural on this. We could remove the ratchetFrom config and do a full
reformat on all branches. The problem is git blame will be useless…
But anyway, if the file has been touched later, git blame will be broken
too.
I saw Andrew has already opened a PR. Let’s get it done.
Thanks.
Bryan Beaudreault <bb...@hubspot.com.invalid>于2022年3月29日 周二00:55写道：
> Thanks for your work here Duo!
>
> What should be our convention on committing unrelated formatting changes
> due to spotless? For example, I have a PR
> https://github.com/apache/hbase/pull/4180 open. I rebased it on latest
> master to pull in spotless and ran `mvn spotless:apply` as suggested. As
> you said in the jira, it properly only applied to the files I touched. But
> some of those files I touched had lots of formatting changes unrelated to
> my changes. I imagine this will make it harder to review.
>
> I wonder if we should do a single pass of spotbugs:apply to the main active
> branches to avoid cases like this. It would probably be a big PR but should
> be relatively safe due to just being formatting. Maybe this was already in
> your plans.
>
> On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <pa...@gmail.com>
> wrote:
>
> > Thanks all for chimming in. HBASE-26617 has been resolved and the patch
> has
> > been committed to branch-2.5+.
> >
> > Will start to work on HBASE-26892 to integrate spotless check in our pre
> > commit build.
> >
> > Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六
> > 02:35写道：
> >
> > > +1, agreed. It would be nice to backport to branch-2 as well so we can
> > keep
> > > the branches somewhat consistent in style at least.
> > >
> > > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <hu...@apache.org>
> > > wrote:
> > >
> > > > +1, a very nice and helpful feature.
> > > >
> > > >
> > > >
> > > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > > > Revive.
> > > > >
> > > > > Will wait for another couple of days, if no big objections, I will
> > move
> > > > > forward to integrate spotless into our active branches.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> > > > >
> > > > > > I've filed HBASE-26617 a while ago and recently I implemented a
> PR
> > > for
> > > > > > master branch.
> > > > > >
> > > > > > https://github.com/apache/hbase/pull/4214
> > <https://github.com/apache/hbase/pull/4214>
> > > > <https://github.com/apache/hbase/pull/4214
> > <https://github.com/apache/hbase/pull/4214>
> > >
> > > > > >
> > > > > > The PR is a bit large because it will also format the pom, the
> > actual
> > > > > > changes are here
> > > > > >
> > > > > >
> > > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > <
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >
> > > > <
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > <
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >
> > > >
> > > > > >
> > > > > > In the PR I make use of the eclipse formatter and import order
> file
> > > to
> > > > > > format our java file. For pom, I just use most of the default
> > > > formatter,
> > > > > > the only exception is to expand empty element.
> > > > > >
> > > > > > The spotless plugin support setting a ratchetFrom option, which
> > could
> > > > be a
> > > > > > commit or a tag. Only files changed after this commit will be
> > > > formatted, so
> > > > > > we can avoid generating a very big patch when running
> > spotless:apply.
> > > > > >
> > > > > > So after we get this in, before submitting a PR, you can just
> type
> > > mvn
> > > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > > automatically. And we could also add a spotless:check step in our
> > pre
> > > > > > commit job, to make sure we run spotless:apply before submitting
> a
> > > PR.
> > > > > >
> > > > > > Just tell me what you think about the above plan. Suggestions are
> > > > always
> > > > > > welcomed.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Bryan Beaudreault <bb...@hubspot.com.INVALID>.
Thanks for your work here Duo!
What should be our convention on committing unrelated formatting changes
due to spotless? For example, I have a PR
https://github.com/apache/hbase/pull/4180 open. I rebased it on latest
master to pull in spotless and ran `mvn spotless:apply` as suggested. As
you said in the jira, it properly only applied to the files I touched. But
some of those files I touched had lots of formatting changes unrelated to
my changes. I imagine this will make it harder to review.
I wonder if we should do a single pass of spotbugs:apply to the main active
branches to avoid cases like this. It would probably be a big PR but should
be relatively safe due to just being formatting. Maybe this was already in
your plans.
On Mon, Mar 28, 2022 at 9:26 AM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> Thanks all for chimming in. HBASE-26617 has been resolved and the patch has
> been committed to branch-2.5+.
>
> Will start to work on HBASE-26892 to integrate spotless check in our pre
> commit build.
>
> Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六
> 02:35写道：
>
> > +1, agreed. It would be nice to backport to branch-2 as well so we can
> keep
> > the branches somewhat consistent in style at least.
> >
> > On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <hu...@apache.org>
> > wrote:
> >
> > > +1, a very nice and helpful feature.
> > >
> > >
> > >
> > > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > > Revive.
> > > >
> > > > Will wait for another couple of days, if no big objections, I will
> move
> > > > forward to integrate spotless into our active branches.
> > > >
> > > > Thanks.
> > > >
> > > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> > > >
> > > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> > for
> > > > > master branch.
> > > > >
> > > > > https://github.com/apache/hbase/pull/4214
> <https://github.com/apache/hbase/pull/4214>
> > > <https://github.com/apache/hbase/pull/4214
> <https://github.com/apache/hbase/pull/4214>
> >
> > > > >
> > > > > The PR is a bit large because it will also format the pom, the
> actual
> > > > > changes are here
> > > > >
> > > > >
> > > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> <https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673>
> > > <
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> <https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673>
> > >
> > > > >
> > > > > In the PR I make use of the eclipse formatter and import order file
> > to
> > > > > format our java file. For pom, I just use most of the default
> > > formatter,
> > > > > the only exception is to expand empty element.
> > > > >
> > > > > The spotless plugin support setting a ratchetFrom option, which
> could
> > > be a
> > > > > commit or a tag. Only files changed after this commit will be
> > > formatted, so
> > > > > we can avoid generating a very big patch when running
> spotless:apply.
> > > > >
> > > > > So after we get this in, before submitting a PR, you can just type
> > mvn
> > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > automatically. And we could also add a spotless:check step in our
> pre
> > > > > commit job, to make sure we run spotless:apply before submitting a
> > PR.
> > > > >
> > > > > Just tell me what you think about the above plan. Suggestions are
> > > always
> > > > > welcomed.
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
Thanks all for chimming in. HBASE-26617 has been resolved and the patch has
been committed to branch-2.5+.
Will start to work on HBASE-26892 to integrate spotless check in our pre
commit build.
Bryan Beaudreault <bb...@hubspot.com.invalid> 于2022年3月26日周六 02:35写道：
> +1, agreed. It would be nice to backport to branch-2 as well so we can keep
> the branches somewhat consistent in style at least.
>
> On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <hu...@apache.org>
> wrote:
>
> > +1, a very nice and helpful feature.
> >
> >
> >
> > On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > > Revive.
> > >
> > > Will wait for another couple of days, if no big objections, I will move
> > > forward to integrate spotless into our active branches.
> > >
> > > Thanks.
> > >
> > > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> > >
> > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> for
> > > > master branch.
> > > >
> > > > https://github.com/apache/hbase/pull/4214
> > <https://github.com/apache/hbase/pull/4214>
> > > >
> > > > The PR is a bit large because it will also format the pom, the actual
> > > > changes are here
> > > >
> > > >
> > > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > <
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >
> > > >
> > > > In the PR I make use of the eclipse formatter and import order file
> to
> > > > format our java file. For pom, I just use most of the default
> > formatter,
> > > > the only exception is to expand empty element.
> > > >
> > > > The spotless plugin support setting a ratchetFrom option, which could
> > be a
> > > > commit or a tag. Only files changed after this commit will be
> > formatted, so
> > > > we can avoid generating a very big patch when running spotless:apply.
> > > >
> > > > So after we get this in, before submitting a PR, you can just type
> mvn
> > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > automatically. And we could also add a spotless:check step in our pre
> > > > commit job, to make sure we run spotless:apply before submitting a
> PR.
> > > >
> > > > Just tell me what you think about the above plan. Suggestions are
> > always
> > > > welcomed.
> > > >
> > > > Thanks.
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Bryan Beaudreault <bb...@hubspot.com.INVALID>.
+1, agreed. It would be nice to backport to branch-2 as well so we can keep
the branches somewhat consistent in style at least.
On Fri, Mar 25, 2022 at 2:05 PM Huaxiang Sun <hu...@apache.org> wrote:
> +1, a very nice and helpful feature.
>
>
>
> On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> > Revive.
> >
> > Will wait for another couple of days, if no big objections, I will move
> > forward to integrate spotless into our active branches.
> >
> > Thanks.
> >
> > 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> >
> > > I've filed HBASE-26617 a while ago and recently I implemented a PR for
> > > master branch.
> > >
> > > https://github.com/apache/hbase/pull/4214
> <https://github.com/apache/hbase/pull/4214>
> > >
> > > The PR is a bit large because it will also format the pom, the actual
> > > changes are here
> > >
> > >
> > >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> <https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673>
> > >
> > > In the PR I make use of the eclipse formatter and import order file to
> > > format our java file. For pom, I just use most of the default
> formatter,
> > > the only exception is to expand empty element.
> > >
> > > The spotless plugin support setting a ratchetFrom option, which could
> be a
> > > commit or a tag. Only files changed after this commit will be
> formatted, so
> > > we can avoid generating a very big patch when running spotless:apply.
> > >
> > > So after we get this in, before submitting a PR, you can just type mvn
> > > spotless:apply, then most of the checkstyle issues will be solved
> > > automatically. And we could also add a spotless:check step in our pre
> > > commit job, to make sure we run spotless:apply before submitting a PR.
> > >
> > > Just tell me what you think about the above plan. Suggestions are
> always
> > > welcomed.
> > >
> > > Thanks.
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Huaxiang Sun <hu...@apache.org>.
+1, a very nice and helpful feature.
On 2022/03/25 04:04:31 "张铎(Duo Zhang)" wrote:
> Revive.
>
> Will wait for another couple of days, if no big objections, I will move
> forward to integrate spotless into our active branches.
>
> Thanks.
>
> 张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
>
> > I've filed HBASE-26617 a while ago and recently I implemented a PR for
> > master branch.
> >
> > https://github.com/apache/hbase/pull/4214
> >
> > The PR is a bit large because it will also format the pom, the actual
> > changes are here
> >
> >
> > https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >
> > In the PR I make use of the eclipse formatter and import order file to
> > format our java file. For pom, I just use most of the default formatter,
> > the only exception is to expand empty element.
> >
> > The spotless plugin support setting a ratchetFrom option, which could be a
> > commit or a tag. Only files changed after this commit will be formatted, so
> > we can avoid generating a very big patch when running spotless:apply.
> >
> > So after we get this in, before submitting a PR, you can just type mvn
> > spotless:apply, then most of the checkstyle issues will be solved
> > automatically. And we could also add a spotless:check step in our pre
> > commit job, to make sure we run spotless:apply before submitting a PR.
> >
> > Just tell me what you think about the above plan. Suggestions are always
> > welcomed.
> >
> > Thanks.
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
Revive.
Will wait for another couple of days, if no big objections, I will move
forward to integrate spotless into our active branches.
Thanks.
张铎(Duo Zhang) <pa...@gmail.com> 于2022年3月15日周二 21:17写道：
> I've filed HBASE-26617 a while ago and recently I implemented a PR for
> master branch.
>
> https://github.com/apache/hbase/pull/4214
>
> The PR is a bit large because it will also format the pom, the actual
> changes are here
>
>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>
> In the PR I make use of the eclipse formatter and import order file to
> format our java file. For pom, I just use most of the default formatter,
> the only exception is to expand empty element.
>
> The spotless plugin support setting a ratchetFrom option, which could be a
> commit or a tag. Only files changed after this commit will be formatted, so
> we can avoid generating a very big patch when running spotless:apply.
>
> So after we get this in, before submitting a PR, you can just type mvn
> spotless:apply, then most of the checkstyle issues will be solved
> automatically. And we could also add a spotless:check step in our pre
> commit job, to make sure we run spotless:apply before submitting a PR.
>
> Just tell me what you think about the above plan. Suggestions are always
> welcomed.
>
> Thanks.
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by tom lee <to...@gmail.com>.
Thanks for all your reply.
张铎(Duo Zhang) <pa...@gmail.com> 于2022年5月11日周三 10:37写道：
> The spotless error is on the master branch, your PR has fixed the error.
>
> tom lee <to...@gmail.com> 于2022年5月11日周三 00:13写道：
>
> > I found some spotless warn, so I repaired them by running "mvn
> > spotless:apply".
> >
> > Then I executed "mvn spotless:check" locally, and the build succeeded,
> but
> > there were still spotless warn after submitting PR
> > https://github.com/apache/hbase/pull/4417.
> >
> > I just found this discussion, so before the end of the discussion, can we
> > not focus on spotless warn?
> >
> > Thanks,
> > Tao Li
> >
> >
> > Peter Somogyi <ps...@apache.org> 于2022年5月10日周二 23:57写道：
> >
> > > I noticed that the spotless license formatting removed some of the
> > license
> > > extensions where the origin of the code was mentioned, credited.
> > > One example is AbstractByteRange [1] which is from Protobuf.
> > >
> > > Are we fine with these removals?
> > > Can someone comment on this who is more familiar with licensing
> > guidelines?
> > >
> > > [1]
> > >
> > >
> >
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
> > >
> > > Thanks,
> > > PEter
> > >
> > > On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org>
> wrote:
> > >
> > > > Ah! Pay heed to the version of the spotless plugin. There was a
> version
> > > > upgrade between what was in the pom file on my feature branch vs. on
> > the
> > > > target branch.
> > > >
> > > > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org>
> > wrote:
> > > >
> > > > > Now that spotless has landed, here's something to help you get your
> > > > > outstanding PRs over the hump. It's not perfect, but maybe it'll
> help
> > > > you.
> > > > >
> > > > > First, get the latest upstream changes and apply the latest
> spotless
> > > > > configuration to your working branch,
> > > > >
> > > > > $ git fetch origin $targetbranch
> > > > > $ git checkout $featurebranch
> > > > > $ git checkout origin/$targetbranch --
> > > > > dev-support/hbase_eclipse_formatter.xml
> > dev-support/eclipse.importorder
> > > > > dev-support/license-header
> > > > >
> > > > > After that, find the parent commit of your changes and drop that
> > value
> > > > > into the ratchedFrom field of pom.xml,
> > > > >
> > > > > $ git rev-parse $featurebranch^
> > > > >
> > > > > Then you can spotless:apply , selectively ammend your commit with
> the
> > > > > results (be sure to omit the files you altered in staging the
> > spotless
> > > > > env), and then rebase. This assumes you have only a single commit
> on
> > > your
> > > > > $featurebranch. if you have more changes, you need the parent of
> your
> > > > first
> > > > > changes on the branch.
> > > > >
> > > > > But I may be missing something. the above isn’t perfect, especially
> > > with
> > > > > javadocs in the likes of HBaseTestingUtility.java that I didn’t
> touch
> > > in
> > > > my
> > > > > original feature branch. My version of spotless appears to disagree
> > > with
> > > > > the committed changes, I see conflicts like:
> > > > >
> > > > > <<<<<<< HEAD
> > > > > * @return A Table instance for the created table. n
> > > > > =======
> > > > > * @return A Table instance for the created table.
> > > > > * @throws IOException
> > > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > > spans)
> > > > >
> > > > > and
> > > > >
> > > > > <<<<<<< HEAD
> > > > > * nnnnn * @return A region on which you must call
> > > > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when
> > done.
> > > n
> > > > > =======
> > > > > * @param tableName
> > > > > * @param startKey
> > > > > * @param stopKey
> > > > > * @param isReadOnly
> > > > > * @param families
> > > > > * @return A region on which you must call {@link
> > > > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > > > * when done.
> > > > > * @throws IOException
> > > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > > spans)
> > > > >
> > > > > Good luck,
> > > > > Nick
> > > > >
> > > > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > > > I've filed HBASE-26617 a while ago and recently I implemented a
> PR
> > > for
> > > > > > master branch.
> > > > > >
> > > > > > https://github.com/apache/hbase/pull/4214
> > > > > >
> > > > > > The PR is a bit large because it will also format the pom, the
> > actual
> > > > > > changes are here
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >
> > > > > > In the PR I make use of the eclipse formatter and import order
> file
> > > to
> > > > > > format our java file. For pom, I just use most of the default
> > > > formatter,
> > > > > > the only exception is to expand empty element.
> > > > > >
> > > > > > The spotless plugin support setting a ratchetFrom option, which
> > could
> > > > be
> > > > > a
> > > > > > commit or a tag. Only files changed after this commit will be
> > > > formatted,
> > > > > so
> > > > > > we can avoid generating a very big patch when running
> > spotless:apply.
> > > > > >
> > > > > > So after we get this in, before submitting a PR, you can just
> type
> > > mvn
> > > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > > automatically. And we could also add a spotless:check step in our
> > pre
> > > > > > commit job, to make sure we run spotless:apply before submitting
> a
> > > PR.
> > > > > >
> > > > > > Just tell me what you think about the above plan. Suggestions are
> > > > always
> > > > > > welcomed.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by "张铎(Duo Zhang)" <pa...@gmail.com>.
The spotless error is on the master branch, your PR has fixed the error.
tom lee <to...@gmail.com> 于2022年5月11日周三 00:13写道：
> I found some spotless warn, so I repaired them by running "mvn
> spotless:apply".
>
> Then I executed "mvn spotless:check" locally, and the build succeeded, but
> there were still spotless warn after submitting PR
> https://github.com/apache/hbase/pull/4417.
>
> I just found this discussion, so before the end of the discussion, can we
> not focus on spotless warn?
>
> Thanks,
> Tao Li
>
>
> Peter Somogyi <ps...@apache.org> 于2022年5月10日周二 23:57写道：
>
> > I noticed that the spotless license formatting removed some of the
> license
> > extensions where the origin of the code was mentioned, credited.
> > One example is AbstractByteRange [1] which is from Protobuf.
> >
> > Are we fine with these removals?
> > Can someone comment on this who is more familiar with licensing
> guidelines?
> >
> > [1]
> >
> >
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
> >
> > Thanks,
> > PEter
> >
> > On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org> wrote:
> >
> > > Ah! Pay heed to the version of the spotless plugin. There was a version
> > > upgrade between what was in the pom file on my feature branch vs. on
> the
> > > target branch.
> > >
> > > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org>
> wrote:
> > >
> > > > Now that spotless has landed, here's something to help you get your
> > > > outstanding PRs over the hump. It's not perfect, but maybe it'll help
> > > you.
> > > >
> > > > First, get the latest upstream changes and apply the latest spotless
> > > > configuration to your working branch,
> > > >
> > > > $ git fetch origin $targetbranch
> > > > $ git checkout $featurebranch
> > > > $ git checkout origin/$targetbranch --
> > > > dev-support/hbase_eclipse_formatter.xml
> dev-support/eclipse.importorder
> > > > dev-support/license-header
> > > >
> > > > After that, find the parent commit of your changes and drop that
> value
> > > > into the ratchedFrom field of pom.xml,
> > > >
> > > > $ git rev-parse $featurebranch^
> > > >
> > > > Then you can spotless:apply , selectively ammend your commit with the
> > > > results (be sure to omit the files you altered in staging the
> spotless
> > > > env), and then rebase. This assumes you have only a single commit on
> > your
> > > > $featurebranch. if you have more changes, you need the parent of your
> > > first
> > > > changes on the branch.
> > > >
> > > > But I may be missing something. the above isn’t perfect, especially
> > with
> > > > javadocs in the likes of HBaseTestingUtility.java that I didn’t touch
> > in
> > > my
> > > > original feature branch. My version of spotless appears to disagree
> > with
> > > > the committed changes, I see conflicts like:
> > > >
> > > > <<<<<<< HEAD
> > > > * @return A Table instance for the created table. n
> > > > =======
> > > > * @return A Table instance for the created table.
> > > > * @throws IOException
> > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > spans)
> > > >
> > > > and
> > > >
> > > > <<<<<<< HEAD
> > > > * nnnnn * @return A region on which you must call
> > > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when
> done.
> > n
> > > > =======
> > > > * @param tableName
> > > > * @param startKey
> > > > * @param stopKey
> > > > * @param isReadOnly
> > > > * @param families
> > > > * @return A region on which you must call {@link
> > > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > > * when done.
> > > > * @throws IOException
> > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > spans)
> > > >
> > > > Good luck,
> > > > Nick
> > > >
> > > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> > for
> > > > > master branch.
> > > > >
> > > > > https://github.com/apache/hbase/pull/4214
> > > > >
> > > > > The PR is a bit large because it will also format the pom, the
> actual
> > > > > changes are here
> > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >
> > > > > In the PR I make use of the eclipse formatter and import order file
> > to
> > > > > format our java file. For pom, I just use most of the default
> > > formatter,
> > > > > the only exception is to expand empty element.
> > > > >
> > > > > The spotless plugin support setting a ratchetFrom option, which
> could
> > > be
> > > > a
> > > > > commit or a tag. Only files changed after this commit will be
> > > formatted,
> > > > so
> > > > > we can avoid generating a very big patch when running
> spotless:apply.
> > > > >
> > > > > So after we get this in, before submitting a PR, you can just type
> > mvn
> > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > automatically. And we could also add a spotless:check step in our
> pre
> > > > > commit job, to make sure we run spotless:apply before submitting a
> > PR.
> > > > >
> > > > > Just tell me what you think about the above plan. Suggestions are
> > > always
> > > > > welcomed.
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by tom lee <to...@gmail.com>.
I found some spotless warn, so I repaired them by running "mvn
spotless:apply".
Then I executed "mvn spotless:check" locally, and the build succeeded, but
there were still spotless warn after submitting PR
https://github.com/apache/hbase/pull/4417.
I just found this discussion, so before the end of the discussion, can we
not focus on spotless warn?
Thanks,
Tao Li
Peter Somogyi <ps...@apache.org> 于2022年5月10日周二 23:57写道：
> I noticed that the spotless license formatting removed some of the license
> extensions where the origin of the code was mentioned, credited.
> One example is AbstractByteRange [1] which is from Protobuf.
>
> Are we fine with these removals?
> Can someone comment on this who is more familiar with licensing guidelines?
>
> [1]
>
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
>
> Thanks,
> PEter
>
> On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org> wrote:
>
> > Ah! Pay heed to the version of the spotless plugin. There was a version
> > upgrade between what was in the pom file on my feature branch vs. on the
> > target branch.
> >
> > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org> wrote:
> >
> > > Now that spotless has landed, here's something to help you get your
> > > outstanding PRs over the hump. It's not perfect, but maybe it'll help
> > you.
> > >
> > > First, get the latest upstream changes and apply the latest spotless
> > > configuration to your working branch,
> > >
> > > $ git fetch origin $targetbranch
> > > $ git checkout $featurebranch
> > > $ git checkout origin/$targetbranch --
> > > dev-support/hbase_eclipse_formatter.xml dev-support/eclipse.importorder
> > > dev-support/license-header
> > >
> > > After that, find the parent commit of your changes and drop that value
> > > into the ratchedFrom field of pom.xml,
> > >
> > > $ git rev-parse $featurebranch^
> > >
> > > Then you can spotless:apply , selectively ammend your commit with the
> > > results (be sure to omit the files you altered in staging the spotless
> > > env), and then rebase. This assumes you have only a single commit on
> your
> > > $featurebranch. if you have more changes, you need the parent of your
> > first
> > > changes on the branch.
> > >
> > > But I may be missing something. the above isn’t perfect, especially
> with
> > > javadocs in the likes of HBaseTestingUtility.java that I didn’t touch
> in
> > my
> > > original feature branch. My version of spotless appears to disagree
> with
> > > the committed changes, I see conflicts like:
> > >
> > > <<<<<<< HEAD
> > > * @return A Table instance for the created table. n
> > > =======
> > > * @return A Table instance for the created table.
> > > * @throws IOException
> > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> spans)
> > >
> > > and
> > >
> > > <<<<<<< HEAD
> > > * nnnnn * @return A region on which you must call
> > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when done.
> n
> > > =======
> > > * @param tableName
> > > * @param startKey
> > > * @param stopKey
> > > * @param isReadOnly
> > > * @param families
> > > * @return A region on which you must call {@link
> > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > * when done.
> > > * @throws IOException
> > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> spans)
> > >
> > > Good luck,
> > > Nick
> > >
> > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> for
> > > > master branch.
> > > >
> > > > https://github.com/apache/hbase/pull/4214
> > > >
> > > > The PR is a bit large because it will also format the pom, the actual
> > > > changes are here
> > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >
> > > > In the PR I make use of the eclipse formatter and import order file
> to
> > > > format our java file. For pom, I just use most of the default
> > formatter,
> > > > the only exception is to expand empty element.
> > > >
> > > > The spotless plugin support setting a ratchetFrom option, which could
> > be
> > > a
> > > > commit or a tag. Only files changed after this commit will be
> > formatted,
> > > so
> > > > we can avoid generating a very big patch when running spotless:apply.
> > > >
> > > > So after we get this in, before submitting a PR, you can just type
> mvn
> > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > automatically. And we could also add a spotless:check step in our pre
> > > > commit job, to make sure we run spotless:apply before submitting a
> PR.
> > > >
> > > > Just tell me what you think about the above plan. Suggestions are
> > always
> > > > welcomed.
> > > >
> > > > Thanks.
> > > >
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Andrew Purtell <ap...@apache.org>.
I see on the issue that Sean indicates we have to put the credit back in
the source file because it is BSD 3-clause. My mistake, I thought PB was
ASL v2.
On Wed, May 11, 2022 at 4:10 AM Peter Somogyi <ps...@apache.org> wrote:
> Thanks for the suggestion, Andrew. I've added it in HBASE-27023.
>
> On Tue, May 10, 2022 at 11:59 PM Andrew Purtell <ap...@apache.org>
> wrote:
>
> > If we credit in the NOTICE file I think it is acceptable.
> >
> > On Tue, May 10, 2022 at 8:57 AM Peter Somogyi <ps...@apache.org>
> wrote:
> >
> > > I noticed that the spotless license formatting removed some of the
> > license
> > > extensions where the origin of the code was mentioned, credited.
> > > One example is AbstractByteRange [1] which is from Protobuf.
> > >
> > > Are we fine with these removals?
> > > Can someone comment on this who is more familiar with licensing
> > guidelines?
> > >
> > > [1]
> > >
> > >
> >
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
> > >
> > > Thanks,
> > > PEter
> > >
> > > On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org>
> wrote:
> > >
> > > > Ah! Pay heed to the version of the spotless plugin. There was a
> version
> > > > upgrade between what was in the pom file on my feature branch vs. on
> > the
> > > > target branch.
> > > >
> > > > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org>
> > wrote:
> > > >
> > > > > Now that spotless has landed, here's something to help you get your
> > > > > outstanding PRs over the hump. It's not perfect, but maybe it'll
> help
> > > > you.
> > > > >
> > > > > First, get the latest upstream changes and apply the latest
> spotless
> > > > > configuration to your working branch,
> > > > >
> > > > > $ git fetch origin $targetbranch
> > > > > $ git checkout $featurebranch
> > > > > $ git checkout origin/$targetbranch --
> > > > > dev-support/hbase_eclipse_formatter.xml
> > dev-support/eclipse.importorder
> > > > > dev-support/license-header
> > > > >
> > > > > After that, find the parent commit of your changes and drop that
> > value
> > > > > into the ratchedFrom field of pom.xml,
> > > > >
> > > > > $ git rev-parse $featurebranch^
> > > > >
> > > > > Then you can spotless:apply , selectively ammend your commit with
> the
> > > > > results (be sure to omit the files you altered in staging the
> > spotless
> > > > > env), and then rebase. This assumes you have only a single commit
> on
> > > your
> > > > > $featurebranch. if you have more changes, you need the parent of
> your
> > > > first
> > > > > changes on the branch.
> > > > >
> > > > > But I may be missing something. the above isn’t perfect, especially
> > > with
> > > > > javadocs in the likes of HBaseTestingUtility.java that I didn’t
> touch
> > > in
> > > > my
> > > > > original feature branch. My version of spotless appears to disagree
> > > with
> > > > > the committed changes, I see conflicts like:
> > > > >
> > > > > <<<<<<< HEAD
> > > > > * @return A Table instance for the created table. n
> > > > > =======
> > > > > * @return A Table instance for the created table.
> > > > > * @throws IOException
> > > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > > spans)
> > > > >
> > > > > and
> > > > >
> > > > > <<<<<<< HEAD
> > > > > * nnnnn * @return A region on which you must call
> > > > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when
> > done.
> > > n
> > > > > =======
> > > > > * @param tableName
> > > > > * @param startKey
> > > > > * @param stopKey
> > > > > * @param isReadOnly
> > > > > * @param families
> > > > > * @return A region on which you must call {@link
> > > > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > > > * when done.
> > > > > * @throws IOException
> > > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > > spans)
> > > > >
> > > > > Good luck,
> > > > > Nick
> > > > >
> > > > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > > > I've filed HBASE-26617 a while ago and recently I implemented a
> PR
> > > for
> > > > > > master branch.
> > > > > >
> > > > > > https://github.com/apache/hbase/pull/4214
> > > > > >
> > > > > > The PR is a bit large because it will also format the pom, the
> > actual
> > > > > > changes are here
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > > >
> > > > > > In the PR I make use of the eclipse formatter and import order
> file
> > > to
> > > > > > format our java file. For pom, I just use most of the default
> > > > formatter,
> > > > > > the only exception is to expand empty element.
> > > > > >
> > > > > > The spotless plugin support setting a ratchetFrom option, which
> > could
> > > > be
> > > > > a
> > > > > > commit or a tag. Only files changed after this commit will be
> > > > formatted,
> > > > > so
> > > > > > we can avoid generating a very big patch when running
> > spotless:apply.
> > > > > >
> > > > > > So after we get this in, before submitting a PR, you can just
> type
> > > mvn
> > > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > > automatically. And we could also add a spotless:check step in our
> > pre
> > > > > > commit job, to make sure we run spotless:apply before submitting
> a
> > > PR.
> > > > > >
> > > > > > Just tell me what you think about the above plan. Suggestions are
> > > > always
> > > > > > welcomed.
> > > > > >
> > > > > > Thanks.
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> > --
> > Best regards,
> > Andrew
> >
> > Unrest, ignorance distilled, nihilistic imbeciles -
> > It's what we’ve earned
> > Welcome, apocalypse, what’s taken you so long?
> > Bring us the fitting end that we’ve been counting on
> > - A23, Welcome, Apocalypse
> >
>
--
Best regards,
Andrew
Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
- A23, Welcome, Apocalypse
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Peter Somogyi <ps...@apache.org>.
Thanks for the suggestion, Andrew. I've added it in HBASE-27023.
On Tue, May 10, 2022 at 11:59 PM Andrew Purtell <ap...@apache.org> wrote:
> If we credit in the NOTICE file I think it is acceptable.
>
> On Tue, May 10, 2022 at 8:57 AM Peter Somogyi <ps...@apache.org> wrote:
>
> > I noticed that the spotless license formatting removed some of the
> license
> > extensions where the origin of the code was mentioned, credited.
> > One example is AbstractByteRange [1] which is from Protobuf.
> >
> > Are we fine with these removals?
> > Can someone comment on this who is more familiar with licensing
> guidelines?
> >
> > [1]
> >
> >
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
> >
> > Thanks,
> > PEter
> >
> > On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org> wrote:
> >
> > > Ah! Pay heed to the version of the spotless plugin. There was a version
> > > upgrade between what was in the pom file on my feature branch vs. on
> the
> > > target branch.
> > >
> > > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org>
> wrote:
> > >
> > > > Now that spotless has landed, here's something to help you get your
> > > > outstanding PRs over the hump. It's not perfect, but maybe it'll help
> > > you.
> > > >
> > > > First, get the latest upstream changes and apply the latest spotless
> > > > configuration to your working branch,
> > > >
> > > > $ git fetch origin $targetbranch
> > > > $ git checkout $featurebranch
> > > > $ git checkout origin/$targetbranch --
> > > > dev-support/hbase_eclipse_formatter.xml
> dev-support/eclipse.importorder
> > > > dev-support/license-header
> > > >
> > > > After that, find the parent commit of your changes and drop that
> value
> > > > into the ratchedFrom field of pom.xml,
> > > >
> > > > $ git rev-parse $featurebranch^
> > > >
> > > > Then you can spotless:apply , selectively ammend your commit with the
> > > > results (be sure to omit the files you altered in staging the
> spotless
> > > > env), and then rebase. This assumes you have only a single commit on
> > your
> > > > $featurebranch. if you have more changes, you need the parent of your
> > > first
> > > > changes on the branch.
> > > >
> > > > But I may be missing something. the above isn’t perfect, especially
> > with
> > > > javadocs in the likes of HBaseTestingUtility.java that I didn’t touch
> > in
> > > my
> > > > original feature branch. My version of spotless appears to disagree
> > with
> > > > the committed changes, I see conflicts like:
> > > >
> > > > <<<<<<< HEAD
> > > > * @return A Table instance for the created table. n
> > > > =======
> > > > * @return A Table instance for the created table.
> > > > * @throws IOException
> > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > spans)
> > > >
> > > > and
> > > >
> > > > <<<<<<< HEAD
> > > > * nnnnn * @return A region on which you must call
> > > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when
> done.
> > n
> > > > =======
> > > > * @param tableName
> > > > * @param startKey
> > > > * @param stopKey
> > > > * @param isReadOnly
> > > > * @param families
> > > > * @return A region on which you must call {@link
> > > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > > * when done.
> > > > * @throws IOException
> > > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> > spans)
> > > >
> > > > Good luck,
> > > > Nick
> > > >
> > > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> > for
> > > > > master branch.
> > > > >
> > > > > https://github.com/apache/hbase/pull/4214
> > > > >
> > > > > The PR is a bit large because it will also format the pom, the
> actual
> > > > > changes are here
> > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > > >
> > > > > In the PR I make use of the eclipse formatter and import order file
> > to
> > > > > format our java file. For pom, I just use most of the default
> > > formatter,
> > > > > the only exception is to expand empty element.
> > > > >
> > > > > The spotless plugin support setting a ratchetFrom option, which
> could
> > > be
> > > > a
> > > > > commit or a tag. Only files changed after this commit will be
> > > formatted,
> > > > so
> > > > > we can avoid generating a very big patch when running
> spotless:apply.
> > > > >
> > > > > So after we get this in, before submitting a PR, you can just type
> > mvn
> > > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > > automatically. And we could also add a spotless:check step in our
> pre
> > > > > commit job, to make sure we run spotless:apply before submitting a
> > PR.
> > > > >
> > > > > Just tell me what you think about the above plan. Suggestions are
> > > always
> > > > > welcomed.
> > > > >
> > > > > Thanks.
> > > > >
> > > >
> > >
> >
>
>
> --
> Best regards,
> Andrew
>
> Unrest, ignorance distilled, nihilistic imbeciles -
> It's what we’ve earned
> Welcome, apocalypse, what’s taken you so long?
> Bring us the fitting end that we’ve been counting on
> - A23, Welcome, Apocalypse
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Andrew Purtell <ap...@apache.org>.
If we credit in the NOTICE file I think it is acceptable.
On Tue, May 10, 2022 at 8:57 AM Peter Somogyi <ps...@apache.org> wrote:
> I noticed that the spotless license formatting removed some of the license
> extensions where the origin of the code was mentioned, credited.
> One example is AbstractByteRange [1] which is from Protobuf.
>
> Are we fine with these removals?
> Can someone comment on this who is more familiar with licensing guidelines?
>
> [1]
>
> https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
>
> Thanks,
> PEter
>
> On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org> wrote:
>
> > Ah! Pay heed to the version of the spotless plugin. There was a version
> > upgrade between what was in the pom file on my feature branch vs. on the
> > target branch.
> >
> > On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org> wrote:
> >
> > > Now that spotless has landed, here's something to help you get your
> > > outstanding PRs over the hump. It's not perfect, but maybe it'll help
> > you.
> > >
> > > First, get the latest upstream changes and apply the latest spotless
> > > configuration to your working branch,
> > >
> > > $ git fetch origin $targetbranch
> > > $ git checkout $featurebranch
> > > $ git checkout origin/$targetbranch --
> > > dev-support/hbase_eclipse_formatter.xml dev-support/eclipse.importorder
> > > dev-support/license-header
> > >
> > > After that, find the parent commit of your changes and drop that value
> > > into the ratchedFrom field of pom.xml,
> > >
> > > $ git rev-parse $featurebranch^
> > >
> > > Then you can spotless:apply , selectively ammend your commit with the
> > > results (be sure to omit the files you altered in staging the spotless
> > > env), and then rebase. This assumes you have only a single commit on
> your
> > > $featurebranch. if you have more changes, you need the parent of your
> > first
> > > changes on the branch.
> > >
> > > But I may be missing something. the above isn’t perfect, especially
> with
> > > javadocs in the likes of HBaseTestingUtility.java that I didn’t touch
> in
> > my
> > > original feature branch. My version of spotless appears to disagree
> with
> > > the committed changes, I see conflicts like:
> > >
> > > <<<<<<< HEAD
> > > * @return A Table instance for the created table. n
> > > =======
> > > * @return A Table instance for the created table.
> > > * @throws IOException
> > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> spans)
> > >
> > > and
> > >
> > > <<<<<<< HEAD
> > > * nnnnn * @return A region on which you must call
> > > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when done.
> n
> > > =======
> > > * @param tableName
> > > * @param startKey
> > > * @param stopKey
> > > * @param isReadOnly
> > > * @param families
> > > * @return A region on which you must call {@link
> > > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > > * when done.
> > > * @throws IOException
> > > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator
> spans)
> > >
> > > Good luck,
> > > Nick
> > >
> > > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > > I've filed HBASE-26617 a while ago and recently I implemented a PR
> for
> > > > master branch.
> > > >
> > > > https://github.com/apache/hbase/pull/4214
> > > >
> > > > The PR is a bit large because it will also format the pom, the actual
> > > > changes are here
> > > >
> > > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > > >
> > > > In the PR I make use of the eclipse formatter and import order file
> to
> > > > format our java file. For pom, I just use most of the default
> > formatter,
> > > > the only exception is to expand empty element.
> > > >
> > > > The spotless plugin support setting a ratchetFrom option, which could
> > be
> > > a
> > > > commit or a tag. Only files changed after this commit will be
> > formatted,
> > > so
> > > > we can avoid generating a very big patch when running spotless:apply.
> > > >
> > > > So after we get this in, before submitting a PR, you can just type
> mvn
> > > > spotless:apply, then most of the checkstyle issues will be solved
> > > > automatically. And we could also add a spotless:check step in our pre
> > > > commit job, to make sure we run spotless:apply before submitting a
> PR.
> > > >
> > > > Just tell me what you think about the above plan. Suggestions are
> > always
> > > > welcomed.
> > > >
> > > > Thanks.
> > > >
> > >
> >
>
--
Best regards,
Andrew
Unrest, ignorance distilled, nihilistic imbeciles -
It's what we’ve earned
Welcome, apocalypse, what’s taken you so long?
Bring us the fitting end that we’ve been counting on
- A23, Welcome, Apocalypse
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Peter Somogyi <ps...@apache.org>.
I noticed that the spotless license formatting removed some of the license
extensions where the origin of the code was mentioned, credited.
One example is AbstractByteRange [1] which is from Protobuf.
Are we fine with these removals?
Can someone comment on this who is more familiar with licensing guidelines?
[1]
https://github.com/apache/hbase/commit/9c8c9e7fbf8005ea89fa9b13d6d063b9f0240443#diff-f5806f14849a23b9265b022f3f330b80d08bcc10fcf69d8ee2e1b0d5af266d52
Thanks,
PEter
On Mon, May 2, 2022 at 4:32 PM Nick Dimiduk <nd...@apache.org> wrote:
> Ah! Pay heed to the version of the spotless plugin. There was a version
> upgrade between what was in the pom file on my feature branch vs. on the
> target branch.
>
> On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org> wrote:
>
> > Now that spotless has landed, here's something to help you get your
> > outstanding PRs over the hump. It's not perfect, but maybe it'll help
> you.
> >
> > First, get the latest upstream changes and apply the latest spotless
> > configuration to your working branch,
> >
> > $ git fetch origin $targetbranch
> > $ git checkout $featurebranch
> > $ git checkout origin/$targetbranch --
> > dev-support/hbase_eclipse_formatter.xml dev-support/eclipse.importorder
> > dev-support/license-header
> >
> > After that, find the parent commit of your changes and drop that value
> > into the ratchedFrom field of pom.xml,
> >
> > $ git rev-parse $featurebranch^
> >
> > Then you can spotless:apply , selectively ammend your commit with the
> > results (be sure to omit the files you altered in staging the spotless
> > env), and then rebase. This assumes you have only a single commit on your
> > $featurebranch. if you have more changes, you need the parent of your
> first
> > changes on the branch.
> >
> > But I may be missing something. the above isn’t perfect, especially with
> > javadocs in the likes of HBaseTestingUtility.java that I didn’t touch in
> my
> > original feature branch. My version of spotless appears to disagree with
> > the committed changes, I see conflicts like:
> >
> > <<<<<<< HEAD
> > * @return A Table instance for the created table. n
> > =======
> > * @return A Table instance for the created table.
> > * @throws IOException
> > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
> >
> > and
> >
> > <<<<<<< HEAD
> > * nnnnn * @return A region on which you must call
> > * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when done. n
> > =======
> > * @param tableName
> > * @param startKey
> > * @param stopKey
> > * @param isReadOnly
> > * @param families
> > * @return A region on which you must call {@link
> > HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> > * when done.
> > * @throws IOException
> > >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
> >
> > Good luck,
> > Nick
> >
> > On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > > I've filed HBASE-26617 a while ago and recently I implemented a PR for
> > > master branch.
> > >
> > > https://github.com/apache/hbase/pull/4214
> > >
> > > The PR is a bit large because it will also format the pom, the actual
> > > changes are here
> > >
> > >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> > >
> > > In the PR I make use of the eclipse formatter and import order file to
> > > format our java file. For pom, I just use most of the default
> formatter,
> > > the only exception is to expand empty element.
> > >
> > > The spotless plugin support setting a ratchetFrom option, which could
> be
> > a
> > > commit or a tag. Only files changed after this commit will be
> formatted,
> > so
> > > we can avoid generating a very big patch when running spotless:apply.
> > >
> > > So after we get this in, before submitting a PR, you can just type mvn
> > > spotless:apply, then most of the checkstyle issues will be solved
> > > automatically. And we could also add a spotless:check step in our pre
> > > commit job, to make sure we run spotless:apply before submitting a PR.
> > >
> > > Just tell me what you think about the above plan. Suggestions are
> always
> > > welcomed.
> > >
> > > Thanks.
> > >
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Nick Dimiduk <nd...@apache.org>.
Ah! Pay heed to the version of the spotless plugin. There was a version
upgrade between what was in the pom file on my feature branch vs. on the
target branch.
On Mon, May 2, 2022 at 4:21 PM Nick Dimiduk <nd...@apache.org> wrote:
> Now that spotless has landed, here's something to help you get your
> outstanding PRs over the hump. It's not perfect, but maybe it'll help you.
>
> First, get the latest upstream changes and apply the latest spotless
> configuration to your working branch,
>
> $ git fetch origin $targetbranch
> $ git checkout $featurebranch
> $ git checkout origin/$targetbranch --
> dev-support/hbase_eclipse_formatter.xml dev-support/eclipse.importorder
> dev-support/license-header
>
> After that, find the parent commit of your changes and drop that value
> into the ratchedFrom field of pom.xml,
>
> $ git rev-parse $featurebranch^
>
> Then you can spotless:apply , selectively ammend your commit with the
> results (be sure to omit the files you altered in staging the spotless
> env), and then rebase. This assumes you have only a single commit on your
> $featurebranch. if you have more changes, you need the parent of your first
> changes on the branch.
>
> But I may be missing something. the above isn’t perfect, especially with
> javadocs in the likes of HBaseTestingUtility.java that I didn’t touch in my
> original feature branch. My version of spotless appears to disagree with
> the committed changes, I see conflicts like:
>
> <<<<<<< HEAD
> * @return A Table instance for the created table. n
> =======
> * @return A Table instance for the created table.
> * @throws IOException
> >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
>
> and
>
> <<<<<<< HEAD
> * nnnnn * @return A region on which you must call
> * {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when done. n
> =======
> * @param tableName
> * @param startKey
> * @param stopKey
> * @param isReadOnly
> * @param families
> * @return A region on which you must call {@link
> HBaseTestingUtility#closeRegionAndWAL(HRegion)}
> * when done.
> * @throws IOException
> >>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
>
> Good luck,
> Nick
>
> On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> > I've filed HBASE-26617 a while ago and recently I implemented a PR for
> > master branch.
> >
> > https://github.com/apache/hbase/pull/4214
> >
> > The PR is a bit large because it will also format the pom, the actual
> > changes are here
> >
> >
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
> >
> > In the PR I make use of the eclipse formatter and import order file to
> > format our java file. For pom, I just use most of the default formatter,
> > the only exception is to expand empty element.
> >
> > The spotless plugin support setting a ratchetFrom option, which could be
> a
> > commit or a tag. Only files changed after this commit will be formatted,
> so
> > we can avoid generating a very big patch when running spotless:apply.
> >
> > So after we get this in, before submitting a PR, you can just type mvn
> > spotless:apply, then most of the checkstyle issues will be solved
> > automatically. And we could also add a spotless:check step in our pre
> > commit job, to make sure we run spotless:apply before submitting a PR.
> >
> > Just tell me what you think about the above plan. Suggestions are always
> > welcomed.
> >
> > Thanks.
> >
>
Re: [DISCUSS] Use spotless to auto format pom and java code
Posted by Nick Dimiduk <nd...@apache.org>.
Now that spotless has landed, here's something to help you get your outstanding PRs over the hump. It's not perfect, but maybe it'll help you.
First, get the latest upstream changes and apply the latest spotless configuration to your working branch,
$ git fetch origin $targetbranch
$ git checkout $featurebranch
$ git checkout origin/$targetbranch -- dev-support/hbase_eclipse_formatter.xml dev-support/eclipse.importorder dev-support/license-header
After that, find the parent commit of your changes and drop that value into the ratchedFrom field of pom.xml,
$ git rev-parse $featurebranch^
Then you can spotless:apply , selectively ammend your commit with the results (be sure to omit the files you altered in staging the spotless env), and then rebase. This assumes you have only a single commit on your $featurebranch. if you have more changes, you need the parent of your first changes on the branch.
But I may be missing something. the above isn’t perfect, especially with javadocs in the likes of HBaseTestingUtility.java that I didn’t touch in my original feature branch. My version of spotless appears to disagree with the committed changes, I see conflicts like:
<<<<<<< HEAD
* @return A Table instance for the created table. n
=======
* @return A Table instance for the created table.
* @throws IOException
>>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
and
<<<<<<< HEAD
* nnnnn * @return A region on which you must call
* {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)} when done. n
=======
* @param tableName
* @param startKey
* @param stopKey
* @param isReadOnly
* @param families
* @return A region on which you must call {@link HBaseTestingUtility#closeRegionAndWAL(HRegion)}
* when done.
* @throws IOException
>>>>>>> 811cfba133 (HBASE-26648 Improve fidelity of RegionLocator spans)
Good luck,
Nick
On 2022/03/15 13:17:32 "张铎(Duo Zhang)" wrote:
> I've filed HBASE-26617 a while ago and recently I implemented a PR for
> master branch.
>
> https://github.com/apache/hbase/pull/4214
>
> The PR is a bit large because it will also format the pom, the actual
> changes are here
>
> https://github.com/apache/hbase/pull/4214/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R2673
>
> In the PR I make use of the eclipse formatter and import order file to
> format our java file. For pom, I just use most of the default formatter,
> the only exception is to expand empty element.
>
> The spotless plugin support setting a ratchetFrom option, which could be a
> commit or a tag. Only files changed after this commit will be formatted, so
> we can avoid generating a very big patch when running spotless:apply.
>
> So after we get this in, before submitting a PR, you can just type mvn
> spotless:apply, then most of the checkstyle issues will be solved
> automatically. And we could also add a spotless:check step in our pre
> commit job, to make sure we run spotless:apply before submitting a PR.
>
> Just tell me what you think about the above plan. Suggestions are always
> welcomed.
>
> Thanks.
>