-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spotless formatting implementation proposal #4408
Comments
If we go for this approach, then I'll add a comment to open PRs pointing them at this issue for the guide on how to update PRs |
I've opened #4409 as a PR for step 1 of the reformatting process. It's fine if there end up being some commits merged between this one (assuming it's accepted, of course) and the big format. In this case, just replace the |
An idea I had from #4414 is that, if automatic reformatting is supported, we could probably add a CI workflow to run the code generators, reformat the code, and fail if the diff is non-empty. This would ensure that everything was generated correctly and that nothing is merged to master that would be overwritten by code generators. |
Background
The lack of automatic formatting in JavaParser makes it quite cumbersome to make any changes that involves code generation. This is because the code generators rewrite the core source files without preserving formatting, resulting in a very large diff with mostly unrelated changes. Since the code style in JP is inconsistent, it's currently impossible to fix this by formatting, so the only solution is to go through the diff and manually add files which contain relevant changes. This is both slow and error-prone (please see #4375 and #4387 for more discussion about this).
In #4202, there was the suggestion to use Spotless for automatic code formatting and I've played around with this for a bit and think I found a way to do this while minimizing impact on currently-open PRs (although there aren't that many in any case and most that exist are fairly small).
Solution
Reformatting JavaParser
Step 1: Add spotless with ratcheting to pom.xml
For this step, the idea is to add the spotless maven plugin configuration to pom.xml with the ratcheting option enabled, but to not run the plugin yet (the ratcheting option here is an option that takes a branch as input (in this case origin/master) and only applies spotless to files that have edits from the target branch). Merge this to master.
Step 2: Fully format JavaParser
Remove the ratcheting locally, re-run the code generators and reformat the entire project. Add spotless:check to the github workflow (possibly removing checkstyle, but there may be regions where they don't overlap). Merge this to master.
Technically removing the ratcheting here isn't necessary since all the regenerated sources would still be reformatted as well, but if we're going to do a huge reformat, then I think it's worth cleaning up everything. The commit from this should also still have ratcheting enabled since that's much faster than reformatting all files every time.
Step 3: Ignore the reformat in git blame
It's possible to ignore commits with git blame, so ignore the big format commit to avoid git-blame always pointing at the person who merged that instead of the person who actually wrote the relevant code. Github supports a
.git-blame-ignore-revs
file by default and this can also be set locally withgit config blame.ignoreRevFile
(which works for intellij as it uses cli git, but I havent' been able to find anything conclusive about eclipse support). See https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view for more details.At this point, the reformatting is done!
Updating open PRs or WIP changes
There may be a better way to do this, but this way worked well during a local test. I tested this with a test formatting branch and the old record-patterns branch, so will give a detailed walkthrough of that here. There are 2 important commits:
Step 1: Squash-rebase changes onto
This step makes dealing with conflicts later much easier. This can easily be done with git cli using:
This shows
by default, but I want to squash everything, so I change that to
before exiting the editor.
Step 2: Amend squashed commit with formatting changes
Step 3: Rebase onto , keeping any changes
Done
From this point on, changes to master can be merged back into the branch as usual
The text was updated successfully, but these errors were encountered: