-
Notifications
You must be signed in to change notification settings - Fork 134
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
Remove support for refaster #2521
Conversation
Generate changelog in
|
Refaster rules, while simpler to write, are substantially more difficult to maintain compared to standard error-prone rules. We no longer write them ourselves, and have recommended against them for the last few years, as it's difficult to tell where they will match, and more difficult to tweak them to avoid edge cases. We expect that removal of refaster will improve build times, especially for excavators. Failures which are caught and ignored compiling refaster rules are a common red herring which point devs in the wrong direction while debugging other issues.
78ec9b3
to
0470563
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I'm supportive.
Do we have any plans to convert some/all of these refaster templates into error-prone checker(s)/fixers?
It's not something I'm currently tracking, but it would be a great project to ramp somebody up on writing error-prone rules. cc @mpritham ;-) |
Clearly this has already happened, but I'm not sure that it's realistic to rewrite refaster rules into error prone rules at scale. They end up being an order of magnitude larger in errorprone. Take for example #2530 which is ~450 lines in error prone but is 36 lines in refaster. We've got a bunch of rules in https://github.com/palantir/assertj-automation/tree/develop/assertj-refaster-rules/src/main/java/com/palantir/assertj/refaster that I value, but now I'm not sure they're worth the effort to port to errorprone. |
I'd consider that a feature, not a bug. The biggest problem with refaster is precision: It's tremendously difficult to gain confidence that it will impact precisely what you want without overreaching or under-reaching. Furthermore refaster rules can't be applied in a way that prevents regressions. |
Refaster rules, while simpler to write, are substantially more difficult to maintain compared to standard error-prone rules. We no longer write them ourselves, and have recommended against them for the last few years, as it's difficult to tell where they will match, and more difficult to tweak them to avoid edge cases.
We expect that removal of refaster will improve build times, especially for excavators. Failures which are caught and ignored compiling refaster rules are a common red herring which point devs in the wrong direction while debugging other issues.
==COMMIT_MSG==
Remove support for refaster
==COMMIT_MSG==