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