I strongly disagree with this. spotless doesn't just standardize formatting, but also checks for a wide variety of code quality issues that people should address. Just because the check is getting ignored and not fixed right away doesn't mean the solution is to remove the check. It means more attention needs to be paid to fix what the check is failing.
Also, spotless depends on compilation step. It's just the entry point into the build, not just a formatting check. If you remove the spotless check without adding a compilation step, then no checks are done at all that verify the code even builds. Also, it's very easy for anybody to fix most spotless checks. The real problem is that people are accepting PRs and merging them when the PR fails the check. The person who made the edit that caused the breakage should fix their PR before accepting it. This isn't just a problem for formatting/style issues. This is a problem with basic compilability. Recently a commit got into the Java code that didn't even compile at all, and ended up needing a new PR to fix it. If the build failure were attended to instead of ignored, that wouldn't have happened. I trivially fixed the spotless check this afternoon in https://github.com/apache/thrift/pull/3019 by simply running `gradle spotlessApply`. It was trivially easy to fix, and anybody could have done it. I'm a strong "no" against removing the spotless check. On Wed, Aug 14, 2024 at 4:53 PM Yuxuan Wang <yuxuan.w...@reddit.com.invalid> wrote: > > I created https://github.com/apache/thrift/pull/3020 as a test to see how > that works. > > That PR also includes changes in https://github.com/apache/thrift/pull/3019, > as otherwise the java lib code won't even build. Which I think shows > another issue with the spotless check currently: it hides bigger problem > that could cause the java lib to stop building, as when spotless check > fails we stops CI early and won't know whether the java lib builds. > > On Wed, Aug 14, 2024 at 9:42 AM Yuxuan Wang <yuxuan.w...@reddit.com> wrote: > > > From my observation of the past few years, every time the github > > dependency bot upgrades the spotless plugin version, it will start to fail > > something that passed before. And it usually takes months for someone to > > fix those, if they are ever fixed. > > > > While having some enforcement on styling is great, making those > > enforcements to break builds (and also block cross tests from running) for > > months is not. If we don't have the resources to keep the styles > > up-to-date, then maybe sacrificing some styling in favor of actual cross > > tests is a better approach? > >
