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) <palomino...@gmail.com> wrote: > +1 > > Andrew Purtell <apurt...@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 <apurt...@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) <palomino...@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 <bbeaudrea...@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) < > palomino...@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 <bbeaudrea...@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 < > > >> huaxiang...@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) <palomino...@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 > > >
