Add css validation #387
Add css validation #387
Conversation
WalkthroughAdds a GitHub Actions workflow that downloads and runs the W3C CSS Validator on Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub
participant Runner as Runner (ubuntu-latest)
participant Checkout as actions/checkout@v4
participant FetchJar as Download css-validator.jar
participant Java as Java (java -jar)
Dev->>GH: push / open PR
GH->>Runner: trigger "css validation"
Runner->>Checkout: checkout repository
Checkout-->>Runner: workspace ready
Runner->>FetchJar: curl/download css-validator.jar
FetchJar-->>Runner: jar saved
Runner->>Java: java -jar css-validator.jar target/css/main.css
Java-->>Runner: validation result (pass/fail)
Runner-->>GH: report job status
sequenceDiagram
participant Dev as Developer
participant Shell as Local Shell / CI
participant NPM as npm (install)
participant Stylelint as node_modules/.bin/stylelint
Dev->>Shell: run `make stylelint`
Shell->>NPM: ensure `npm install` (provides local binaries)
NPM-->>Shell: local binaries present
Shell->>Stylelint: run `./node_modules/.bin/stylelint --fix sass/*.scss`
Stylelint-->>Shell: lint results / fixes
Shell-->>Dev: exit code / output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/csscheck.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: copyrights
.github/workflows/csscheck.yml
[warning] 1-1: Missed: CSS check workflow was not triggered (csscheck.yml).
🪛 GitHub Actions: yamllint
.github/workflows/csscheck.yml
[warning] 1-1: YAML lint warning: missing document start '---' (document-start).
[warning] 3-3: YAML lint warning: truthy value should be one of [false, true] (truthy).
[error] 9-9: YAML lint error: wrong indentation: expected 6 but found 4 (indentation).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: make
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/csscheck.yml (1)
1-1: Add YAML document start marker.The yamllint tool expects YAML files to start with the
---document start marker.Apply this diff:
+--- name: css validation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/csscheck.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: copyrights
.github/workflows/csscheck.yml
[warning] 1-1: Missed: CSS checks workflow (csscheck.yml) was not executed.
🪛 GitHub Actions: yamllint
.github/workflows/csscheck.yml
[warning] 1-1: yamllint: missing document start '---'.
[warning] 3-3: yamllint: truthy value should be one of [false, true].
[error] 3-3: yamllint: trailing spaces.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: make
|
@kodsurfer what's with this one? ready to finish it? |
|
@yegor256 work in progress, I hope to finish soon |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/csscheck.yml (1)
15-18: Add Java version specification and input validation.The workflow uses
javawithout specifying a version, which can lead to unpredictable behavior if the Java environment changes. Consider setting up a specific Java version. Additionally, add validation to ensure the CSS file exists and provide clear error messages if prerequisites are missing.steps: - uses: actions/checkout@v4 + - name: Set up Java + uses: actions/setup-java@v4 + with: + java-version: '11' + distribution: 'temurin' - name: Validate CSS with W3C Jar run: | wget -O css-validator.jar \ https://github.com/w3c/css-validator/releases/download/cssval-20250226/css-validator.jar + + # Fail fast if CSS file is missing + if [ ! -f target/css/main.css ]; then + echo "Error: target/css/main.css not found. Ensure the CSS build step runs before validation." + exit 1 + fi + java -jar css-validator.jar file:target/css/main.css
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/csscheck.yml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: yamllint
.github/workflows/csscheck.yml
[warning] 6-6: truthy value should be one of [false, true] (truthy)
[error] 17-17: line too long (124 > 80 characters) (line-length)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: make
🔇 Additional comments (1)
.github/workflows/csscheck.yml (1)
6-8: Verify event trigger configuration.The yamllint tool reports a truthy value warning on line 6. The explicit object syntax for event triggers is correct; however, confirm there is no trailing whitespace or hidden characters on the
on:line that may trigger this warning.Run the following script to verify the YAML structure and diagnose the truthy warning:
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: css validation
Makefile
[error] 70-70: Makefile:70: node_modules/.bin/stylelint failed with exit code 254
🪛 GitHub Actions: make
Makefile
[error] 70-70: Make target 'node_modules/.bin/stylelint' failed. Command 'make' failed at Makefile:70 with exit code 254.
| node_modules/.bin/stylelint: | ||
| npm install | ||
|
|
||
| stylelint: node_modules/.bin/stylelint | ||
| ./node_modules/.bin/stylelint stylelint sass/*.scss --fix |
There was a problem hiding this comment.
Fix incorrect Make target declaration and malformed stylelint command.
The current implementation has two critical issues causing the pipeline failure (exit code 254):
-
Line 71:
node_modules/.bin/stylelintis declared as a target but not marked.PHONY. Make treats non-phony targets as files to be created; the recipenpm installdoesn't directly create this file path, causing Make to incorrectly determine whether the target was successfully built. -
Line 75: The stylelint command has a malformed argument:
stylelint stylelint sass/*.scss --fix. The secondstylelintis being passed as an argument to the executable (treating it as a file to lint), rather than being part of the recipe itself.
Use one of these corrected approaches:
Option 1 (Recommended): Track npm_modules via package.json
-node_modules/.bin/stylelint:
- npm install
-
-stylelint: node_modules/.bin/stylelint
- ./node_modules/.bin/stylelint stylelint sass/*.scss --fix
+node_modules: package.json
+ npm install
+
+stylelint: node_modules
+ ./node_modules/.bin/stylelint sass/*.scss --fixThis ensures npm install only runs when dependencies change.
Option 2 (If npm_modules should always be checked): Use .PHONY with explicit check
-.PHONY: clean all assets install rake stylelint test entries
+.PHONY: clean all assets install rake stylelint test entries node_modules
-node_modules/.bin/stylelint:
+node_modules:
npm install
-
-stylelint: node_modules/.bin/stylelint
- ./node_modules/.bin/stylelint stylelint sass/*.scss --fix
+
+stylelint: node_modules
+ ./node_modules/.bin/stylelint sass/*.scss --fix📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| node_modules/.bin/stylelint: | |
| npm install | |
| stylelint: node_modules/.bin/stylelint | |
| ./node_modules/.bin/stylelint stylelint sass/*.scss --fix | |
| node_modules: package.json | |
| npm install | |
| stylelint: node_modules | |
| ./node_modules/.bin/stylelint sass/*.scss --fix |
🤖 Prompt for AI Agents
In Makefile around lines 71 to 75, the Make target and stylelint invocation are
wrong: mark the prerequisite that doesn't correspond to a file as .PHONY or
(preferably) track node_modules via package.json, and fix the malformed
stylelint command that passes "stylelint" as an argument. Fix by replacing the
current target with one of two approaches: Option 1 (recommended) — declare a
target that depends on package.json (or package-lock.json/yarn.lock) so npm
install runs only when deps change, e.g. target: package.json -> run npm ci/ npm
install, then run the stylelint binary path with the correct files list (remove
the extra "stylelint" token) and --fix; Option 2 — if you must keep a phony
target for node_modules, add .PHONY: node_modules/.bin/stylelint and update the
recipe to explicitly check/install (npm ci) then invoke
./node_modules/.bin/stylelint sass/*.scss --fix (no extra "stylelint" token).
2 participants
Summary by CodeRabbit