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) <[email protected]> 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 <[email protected]> 于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 <[email protected]>
> > 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) <[email protected]> 于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.
> > > > >
> > > >
> > >
> >
>

Reply via email to