ci(lint): Replace deprecated Stylelint Github formatter with a Github problem matcher. #112
Conversation
…ng the previous formatter approach.
WalkthroughThis pull request introduces a new JSON configuration file for Stylelint problem matching, modifies the GitHub Actions workflow for linting to integrate this matcher, and simplifies the linting command in the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
.github/workflows/lint.yaml (1)
34-38: Consider adding error handling for the lint scriptWhile the setup is good, consider adding error handling to ensure the script fails gracefully and provides clear error messages.
- name: "Run `lint:ci` script." run: |- # Disable ANSI color codes in Stylelint default "string" formatter export NO_COLOR=1 - npm run lint:ci + npm run lint:ci || { + echo "::error::Linting failed. Please check the logs above for details." + exit 1 + }package.json (1)
14-14: LGTM! The change aligns with the new problem matcher approach.The simplified
lint:cicommand works well with the new Stylelint problem matcher configuration, removing the need for specific formatters.Consider reusing the existing
lint:checkscript to avoid potential maintenance issues:- "lint:ci": "npm-run-all --sequential --continue-on-error lint:check:*", + "lint:ci": "npm run lint:check",This would ensure both commands stay in sync if the lint checks are modified in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/problem-matchers/stylelint.json(1 hunks).github/workflows/lint.yaml(1 hunks)package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/problem-matchers/stylelint.json
🔇 Additional comments (2)
.github/workflows/lint.yaml (2)
39-41: Well-implemented cleanup step!
The cleanup step is properly configured with if: always() to ensure the problem matcher is removed regardless of the linting outcome. This is excellent practice for maintaining a clean GitHub Actions environment.
32-33: Verify the existence of the problem matcher JSON file
The step assumes the existence of .github/problem-matchers/stylelint.json. Let's verify this dependency.
✅ Verification successful
Problem matcher JSON file exists and is properly formatted
The .github/problem-matchers/stylelint.json file exists and contains a valid problem matcher configuration with proper regex patterns for parsing Stylelint output. The file structure follows GitHub Actions' problem matcher schema.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and validity of the Stylelint problem matcher JSON file
# Test: Check if the file exists and is valid JSON
if [ -f ".github/problem-matchers/stylelint.json" ]; then
echo "File exists. Validating JSON..."
jq '.' .github/problem-matchers/stylelint.json
else
echo "ERROR: Problem matcher JSON file not found!"
exit 1
fi
Length of output: 550
.github/workflows/lint.yaml
Outdated
| @@ -29,4 +29,13 @@ jobs: | |||
| with: | |||
| node-version: 22 | |||
| - run: "npm clean-install" | |||
| - run: "npm run lint:ci" | |||
| - name: "Set up Stylelint Problem Matcher" | |||
| run: "echo '::add-matcher::./.github/problem-matchers/stylelint.json'" | |||
There was a problem hiding this comment.
Should we add a variable for the file path? and reuse it later?
…uggestions from code review Co-authored-by: davemarco <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/lint.yaml (1)
32-33: Consider using an environment variable for the problem matcher pathTo improve maintainability and follow the DRY principle, consider defining the problem matcher path as an environment variable that can be reused in both setup and cleanup steps.
+ - name: "Set problem matcher path" + run: echo "PROBLEM_MATCHER_PATH=./.github/problem-matchers/stylelint.json" >> $GITHUB_ENV - name: "Set up Stylelint Problem Matcher" - run: "echo '::add-matcher::./.github/problem-matchers/stylelint.json'" + run: "echo '::add-matcher::${{ env.PROBLEM_MATCHER_PATH }}'"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/lint.yaml(1 hunks)
🔇 Additional comments (2)
.github/workflows/lint.yaml (2)
34-38: Well-structured lint execution step
The step is well-organized with:
- Clear step name
- Helpful comment explaining the NO_COLOR environment variable
- Proper command formatting
39-41: Properly implemented cleanup step
The cleanup step follows best practices by:
- Using if: "always()" to ensure cleanup runs
- Including the owner parameter for proper matcher removal
- Having a clear, descriptive name
junhaoliao
changed the title
# Conflicts: # package.json
junhaoliao
changed the title
|
@davemarco thanks for the review. Could you take a look again at the code and also the PR title? |
|
Do you like |
junhaoliao
changed the title
References
See #110 .
Description
lint:ciscript inpackage.json.Validation performed
mainbranch and observed the workflow ran and the errors were labelled inline: https://github.com/junhaoliao/yscope-log-viewer/pull/11/filesSummary by CodeRabbit
New Features
Improvements
These updates aim to improve the efficiency and clarity of style issue identification within the codebase.