Jackie-Jiang commented on code in PR #18361: URL: https://github.com/apache/pinot/pull/18361#discussion_r3156403424
########## .claude/skills/lint/SKILL.md: ########## @@ -15,21 +15,22 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -name: precommit -description: Run Pinot's mandatory pre-commit checks (spotless, license, checkstyle) on only the modules affected by the current diff. Auto-fixes what it can. +name: lint +description: Run all Pinot pre-commit quality checks (formatting, license, style, compiler warnings) on modules affected by the current diff. Auto-fixes what it can, reports what it can't. --- -# /precommit +# /lint -Purpose: before pushing a commit or opening a PR, run the four mandatory pre-commit checks from `CLAUDE.md` on the modules the current diff actually touches. Don't run them on the whole repo — that's slow and wasteful on a tree this size. +Purpose: before pushing a commit or opening a PR, run all quality checks on the modules the current diff actually touches. Don't run them on the whole repo — that's slow and wasteful on a tree this size. -The four checks (in order): +The five checks (in order): 1. `./mvnw spotless:apply -pl <modules>` — auto-formats code. 2. `./mvnw license:format -pl <modules>` — adds ASF headers to any new files. 3. `./mvnw checkstyle:check -pl <modules>` — validates style; fails hard. 4. `./mvnw license:check -pl <modules>` — validates headers; fails hard. +5. `./mvnw test-compile -pl <modules> -am -Dmaven.compiler.showDeprecation=true -Dmaven.compiler.showWarnings=true '-Dmaven.compiler.compilerArgs=-Xlint:all'` — compiles and checks for deprecation, unchecked casts, raw types, fallthrough, etc. Warnings are filtered to only changed files. -Steps 1 and 2 are auto-fixers. Steps 3 and 4 are validators — if they fail after the auto-fixers ran, report the failure with the exact offending file/line from the Maven output and stop. Do not try to manually patch style errors; fix the underlying issue or ask the user. +Steps 1 and 2 are auto-fixers. Steps 3 and 4 are validators. Step 5 is a compiler check — if it produces warnings in changed files, report them and stop. Do not try to suppress warnings with `@SuppressWarnings`; fix the underlying issue or ask the user. Review Comment: Do not suppress warnings is not always possible, e.g. certain raw type handling, e.g. `Comparable`. We need to understand the impact of this extra check, and the overhead. How long does it take to execute this on all modules? How many warnings does it find with existing code? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
