Hmm. It would be unfortunate to make contributors include unrelated style changes in their PRs. This would be especially hard on new contributors who might not want to make a large change.
If we really want to do something like this, I would vote for A1. Just do the change all at once and get it over with. I'm also curious what benefit we get out of making these changes. If the code style was acceptable to the reviewers who committed the code, maybe we should leave it alone? best, Colin On Tue, Aug 7, 2018, at 09:41, Guozhang Wang wrote: > Hello Ray, > > I saw on the original PR Jason (cc'ed) expressed a concern comparing > scalafmt with scalastyle: the latter will throw exceptions in the build > process to notify developers while the former will automatically reformat > the code that developers may not be aware of. So I think maybe Jason can > elaborate a bit more of your thoughts on this regard. > > Personally I like this idea (scalafmt). As for cherry-picking burdens, > there may be always some unavoidable, and I think B4 seems less invasive > and hence preferable. > > > Guozhang > > > > > On Mon, Jul 30, 2018 at 1:20 PM, Ray Chiang <rchi...@apache.org> wrote: > > > I had started on KAFKA-2423 (was Scalastyle, now Expand scalafmt to > > core). As part of the cleanup, applying the "gradlew spotlessApply" > > command ended up affecting too many (435 out of 439) files. Since this > > will affect every file, this sort of change does risk polluting the git > > logs. > > > > So, I'd like to get a discussion going to find some agreement on an > > approach. Right now, I see two categories of options: > > > > A) Getting scalafmt working on the existing code > > B) Getting all the code conforming to scalafmt requirements > > > > For the first, I see a couple of approaches: > > > > A1) Do the minimum change that allows scalafmt to run on all the .scala > > files > > A2) Make the change so that scalafmt runs as-is (only on the streams code) > > and add a different task/options that allow running scalafmt on a subset of > > code. (Reasons explained below) > > > > For the second, I can think of the following options: > > > > B1) Do one giant git commit of all cleaned code (no one seemed to like > > this) > > B2) Do git commits one file at a time (trunk or as a branch) > > B3) Do git commits one leaf subdirectory at a time (trunk or as a branch) > > B4) With each pull request on all patches, run option A2) on the affected > > files > > > > From what I can envision, options B2 and B3 require quite a bit of manual > > work if we want to cover multiple releases. The "cleanest" option I can > > think of looks something like: > > > > C1) Contributor makes code modifications for their JIRA > > C2) Contributor runs option A2 to also apply scalafmt to their existing > > code > > C3) Committer does the regular review process > > > > At some point in the future, enough cleanup could be done that the final > > cleanup can be done as a much smaller set of MINOR commits. > > > > -Ray > > > > > > > -- > -- Guozhang
