jdaugherty commented on PR #14925: URL: https://github.com/apache/grails-core/pull/14925#issuecomment-3221532628
I suggest we remove the gradle codenarc / checkstyle config, update the ignored rev list, squash, and merge this PR so we have a starting point on styling. You've done amazing work on this PR and I think it's important to merge. Concerning the workaround of `./gradlew test iT`, I don't see how that's equivalent to running the `build` task. Build will run other tasks such as process resources & execute our gradle plugins because of the functional test apps. One of the reasons we left gradle plugins applied in the test apps was to ensure their logic is executed, but those tasks won't work if the styling isn't perfect since they aren't a part of an integration run. Styling enforcement then could act as a delay when developing locally. Concerning turning on enforcement, I'm against this implementation because it will interfere during periods of rapid, local development. We also need to be able to easily identify when a PR fails for a style issue vs logic issue. Having a disable option allows both development styles & it could support a separate workflow to point out styling issues. While integrating prevents that flexibility so I don't see a downside to supporting both vs always enforcing. If someone prefers to enforce always, they can leave it enabled. I also think we need an autoformat option - like spotless to minimize the styling rules impacts for new contributors & to reduce errors. Do we have a solution for that? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@grails.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
