[PATCH] Implemented validation logic for #196 #227
Conversation
Changes in file .github/workflows/makefile-lint.yml: - implemented check for validating YAML_ARGS Changes in file .github/workflows/markdown-lint.yml: - implemented check for validating YAML_ARGS
WalkthroughThe pull request introduces updates to the GitHub workflows for linting Makefiles and Markdown files. Key changes include the addition of a Changes
Possibly related issues
Possibly related PRs
Suggested labels
Poem
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
Documentation and Community
|
github-actions
bot
added
Testing
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #227 +/- ##
=========================================
Coverage 99.306% 99.306%
=========================================
Files 7 7
Lines 432 432
Branches 31 31
=========================================
Hits 429 429
Misses 2 2
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Changes in file .github/workflows/makefile-lint.yml: - Use POSIX-compliant validation - Provide more detailed error messages - Add debug logging (safely) Changes in file .github/workflows/markdown-lint.yml: - fixed incorect file path - Use POSIX-compliant validation - Provide more detailed error messages - Add debug logging (safely)
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…- WIP #227 -) Changes in file .github/workflows/makefile-lint.yml: - fixed regression by correctly setting `YAML_ARGS="${{ vars.YAML_ARGS }}"` as inconsistant use of `"${{ vars.YAML_ARGS }}"` and `${YAML_ARGS}` caused the regression. Changes in file .github/workflows/markdown-lint.yml: - fixed regression by correctly setting `YAML_ARGS="${{ vars.YAML_ARGS }}"` as inconsistant use of `"${{ vars.YAML_ARGS }}"` and `${YAML_ARGS}` caused the regression.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Additions with file package.json: - new file to track all NPM dependencies in CI. Changes in file .github/dependabot.yml: - updated to auto-update NPM stuff. Changes in file .github/workflows/markdown-lint.yml: - test npm via packages.json setup.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Changes in file .github/workflows/markdown-lint.yml: - Possible fix for missing markdownlint with local install. Changes in file package.json: - added script config
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Changes in file .github/workflows/markdown-lint.yml: - applied changes as discussed in review of PR #227
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Changes in file .github/workflows/markdown-lint.yml: - fixed regex - fixed error message
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/markdown-lint.yml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/markdown-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
📓 Learnings (2)
📓 Common learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers scripts to include explicit data processing steps that produce the expected output, ensuring accurate and useful results.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:50.461Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T04:45:25.464Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, avoiding raw source code dumps, and focusing on clear explanations and real-world examples.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:55.563Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2024-11-14T07:03:35.207Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-12-02T08:20:59.928Z
Learning: When implementing package.json, reactive-firewall prefers to maintain flexibility in npm installation methods, supporting both global (-g) installations and local project-specific installations.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T01:10:18.136Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, following the project's style and conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2024-11-15T19:32:16.878Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#110
File: docs/USAGE.md:66-66
Timestamp: 2024-11-12T10:29:55.563Z
Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Caution".
.github/workflows/markdown-lint.yml (2)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
🪛 actionlint (1.7.4)
.github/workflows/markdown-lint.yml
35-35: shellcheck reported issue in this script: SC2182:error:8:12: This printf format string has no variables. Other arguments are ignored
(shellcheck)
🔇 Additional comments (2)
.github/workflows/markdown-lint.yml (2)
52-60: LGTM! Well-implemented error handling.
The error handling for npm installation is well-implemented with:
- Proper error message capture using command substitution
- Clear error reporting with specific locations
- Good variable naming following project conventions
70-70: LGTM! Clean implementation of markdown linting.
The command properly uses npm scripts and handles file paths with spaces correctly.
Changes in file .github/workflows/markdown-lint.yml: - fixed line numbers and stuff
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/markdown-lint.yml (1)
71-71: Remove unnecessary trailing semicolon.The trailing semicolon after the npm command is not needed.
- xargs -0 -I{} npm run lint:md -- "{}"; + xargs -0 -I{} npm run lint:md -- "{}"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/markdown-lint.yml(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/markdown-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
📓 Learnings (2)
📓 Common learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers scripts to include explicit data processing steps that produce the expected output, ensuring accurate and useful results.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:50.461Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T04:45:25.464Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, avoiding raw source code dumps, and focusing on clear explanations and real-world examples.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:55.563Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2024-11-14T07:03:35.207Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-12-02T08:20:59.928Z
Learning: When implementing package.json, reactive-firewall prefers to maintain flexibility in npm installation methods, supporting both global (-g) installations and local project-specific installations.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T01:10:18.136Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, following the project's style and conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2024-11-15T19:32:16.878Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#110
File: docs/USAGE.md:66-66
Timestamp: 2024-11-12T10:29:55.563Z
Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Caution".
.github/workflows/markdown-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:37-37
Timestamp: 2024-12-03T03:18:50.682Z
Learning: In `.github/workflows/markdown-lint.yml`, reactive-firewall intentionally uses the regex pattern `if [[ "${YAML_ARGS}" =~ ^*[!a-zA-Z0-9\ \-\.]*$ ]]; then` for validation and prefers to keep it as is.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
🪛 GitHub Check: markdown-lint
.github/workflows/markdown-lint.yml
[notice] 36-47: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
🔇 Additional comments (3)
.github/workflows/markdown-lint.yml (3)
34-34: LGTM! Good addition of conditional execution.
Adding the success condition ensures the step only runs when previous steps have completed successfully, which is a good practice for workflow reliability.
36-48: Well-implemented YAML arguments validation and handling!
The implementation includes:
- Secure validation of YAML_ARGS
- Clear error messaging with proper GitHub Actions annotations
- Safe argument splitting using
read -ra
🧰 Tools
🪛 GitHub Check: markdown-lint
[notice] 36-47: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
52-61: Excellent error handling implementation for npm install!
The error handling:
- Captures npm's actual error output
- Reports errors with proper context and location
- Maintains clean error message formatting
Changes in file .github/workflows/makefile-lint.yml: - improve YAML_ARGS validation logic Changes in file .github/workflows/markdown-lint.yml: - improve error message construction
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/makefile-lint.yml(3 hunks).github/workflows/markdown-lint.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/makefile-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
.github/workflows/markdown-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
📓 Learnings (3)
📓 Common learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers scripts to include explicit data processing steps that produce the expected output, ensuring accurate and useful results.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:50.461Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T04:45:25.464Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, avoiding raw source code dumps, and focusing on clear explanations and real-world examples.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:55.563Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2024-11-14T07:03:35.207Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-12-02T08:20:59.928Z
Learning: When implementing package.json, reactive-firewall prefers to maintain flexibility in npm installation methods, supporting both global (-g) installations and local project-specific installations.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T01:10:18.136Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, following the project's style and conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2024-11-15T19:32:16.878Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#110
File: docs/USAGE.md:66-66
Timestamp: 2024-11-12T10:29:55.563Z
Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Caution".
.github/workflows/makefile-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:34-39
Timestamp: 2024-11-29T09:46:47.010Z
Learning: For future scripts in workflow files (e.g., `.github/workflows/makefile-lint.yml`), reactive-firewall prefers concise code and avoids unnecessary variable assignments when possible, such as using `${{ vars.YAML_ARGS }}` directly without assigning it to another variable.
.github/workflows/markdown-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:37-37
Timestamp: 2024-12-03T03:18:50.682Z
Learning: In `.github/workflows/markdown-lint.yml`, reactive-firewall intentionally uses the regex pattern `if [[ "${YAML_ARGS}" =~ ^*[!a-zA-Z0-9\ \-\.]*$ ]]; then` for validation and prefers to keep it as is.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
🪛 actionlint (1.7.4)
.github/workflows/markdown-lint.yml
70-70: shellcheck reported issue in this script: SC2034:warning:2:10: ARGS appears unused. Verify use (or export if used externally)
(shellcheck)
🪛 GitHub Check: markdown-lint
.github/workflows/markdown-lint.yml
[notice] 42-43: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
🪛 GitHub Check: makefile-lint
.github/workflows/markdown-lint.yml
[notice] 36-47: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
🔇 Additional comments (6)
.github/workflows/markdown-lint.yml (3)
26-37: LGTM! Well-structured error handling implementation.
The print_error function provides consistent error reporting and the installation checks are properly validated.
40-54: LGTM! Secure YAML_ARGS validation.
The implementation correctly:
- Validates input using the intended regex pattern
- Safely splits arguments using
read -ra - Uses array expansion for secure argument passing
🧰 Tools
🪛 GitHub Check: markdown-lint
[notice] 42-43: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
58-67: LGTM! Robust NPM installation error handling.
The implementation:
- Captures npm's error output
- Reports errors with proper context
- Maintains clean error reporting structure
.github/workflows/makefile-lint.yml (3)
2-16: LGTM! Well-documented function.
The documentation for print_error follows CEP-7 guidelines and provides clear usage instructions.
41-52: LGTM! Robust dependency validation.
The implementation properly validates all required dependencies with clear error reporting.
114-127: LGTM! Proper error handling for makefile linting.
The implementation includes:
- Proper validation of tool path
- Clear error reporting with context
- Safe handling of file paths
Changes in file .github/workflows/makefile-lint.yml: - fixed line numebrs in diognostics Changes in file .github/workflows/markdown-lint.yml: - fixed a typo - fixed line numbers in diognostics
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/markdown-lint.yml (2)
26-37: Consider enhancing apt-get error handling.While the error handling is good, the immediate exit on apt-get update failure could be too strict. Consider capturing and reporting the update error instead.
- sudo apt-get update || exit 1 + sudo apt-get update || { print_error "Package update failed." 26 26; exit 1; }
77-81: Add error handling for file discovery.The markdown file discovery process lacks error handling.
run: | + if ! FILES=$(git ls-files --exclude-standard -z -- *.md **/*.md 2>&1); then + printf "::error file=.github/workflows/markdown-lint.yml,line=80,endLine=80,title=FILE_ERROR::Failed to list markdown files: %s\n" "${FILES}" + exit 1 + fi - git ls-files --exclude-standard -z -- *.md **/*.md |\ - xargs -0 -I{} npm run lint:md -- "{}"; + echo "${FILES}" | xargs -0 -I{} npm run lint:md -- "{}" || exit 1.github/workflows/makefile-lint.yml (1)
114-127: Consider improving error handling and reducing duplication.
- The
print_errorfunction is duplicated. Consider moving it to a shared script.- The linting error handling could be more detailed.
- "$TOOL_PATH" "${FILE}" || { print_error "Linting failed." 127 127; exit 1; } ; + if ! LINT_ERROR=$("$TOOL_PATH" "${FILE}" 2>&1); then + print_error "Linting failed: ${LINT_ERROR}" 127 127 + exit 1 + fi
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/makefile-lint.yml(4 hunks).github/workflows/markdown-lint.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/makefile-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
.github/workflows/markdown-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
📓 Learnings (3)
📓 Common learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers scripts to include explicit data processing steps that produce the expected output, ensuring accurate and useful results.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:50.461Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T04:45:25.464Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, avoiding raw source code dumps, and focusing on clear explanations and real-world examples.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:55.563Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2024-11-14T07:03:35.207Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-12-02T08:20:59.928Z
Learning: When implementing package.json, reactive-firewall prefers to maintain flexibility in npm installation methods, supporting both global (-g) installations and local project-specific installations.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T01:10:18.136Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, following the project's style and conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2024-11-15T19:32:16.878Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#110
File: docs/USAGE.md:66-66
Timestamp: 2024-11-12T10:29:55.563Z
Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Caution".
.github/workflows/makefile-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:34-39
Timestamp: 2024-11-29T09:46:47.010Z
Learning: For future scripts in workflow files (e.g., `.github/workflows/makefile-lint.yml`), reactive-firewall prefers concise code and avoids unnecessary variable assignments when possible, such as using `${{ vars.YAML_ARGS }}` directly without assigning it to another variable.
.github/workflows/markdown-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:37-37
Timestamp: 2024-12-03T03:18:50.682Z
Learning: In `.github/workflows/markdown-lint.yml`, reactive-firewall intentionally uses the regex pattern `if [[ "${YAML_ARGS}" =~ ^*[!a-zA-Z0-9\ \-\.]*$ ]]; then` for validation and prefers to keep it as is.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
🪛 GitHub Check: makefile-lint
.github/workflows/makefile-lint.yml
[notice] 67-67: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
🪛 GitHub Check: markdown-lint
.github/workflows/markdown-lint.yml
[notice] 53-53: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
🔇 Additional comments (6)
.github/workflows/markdown-lint.yml (3)
28-35: Well-structured error reporting function!
The print_error function follows best practices with proper parameter handling and POSIX-compliant output formatting.
42-54: Secure and well-structured YAML_ARGS validation!
The implementation properly validates input and safely handles arguments using an array.
🧰 Tools
🪛 GitHub Check: markdown-lint
[notice] 53-53: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
66-75: Excellent NPM error handling implementation!
The error handling captures and reports npm errors effectively, with proper context and clear messaging.
.github/workflows/makefile-lint.yml (3)
3-16: Excellent function documentation!
The documentation follows CEP-7 guidelines and provides clear, comprehensive information about the function's purpose and usage.
41-52: Consider enhancing apt-get error handling.
While the error handling is good, the immediate exit on apt-get update failure could be too strict. Consider capturing and reporting the update error instead.
56-68: Consistent YAML_ARGS validation implementation!
The validation logic matches the markdown-lint workflow, maintaining consistency across the codebase.
🧰 Tools
🪛 GitHub Check: makefile-lint
[notice] 67-67: Validation
Validating workflow with args: -f github --config-file .yamllint.conf --no-warnings
Changes in file .github/workflows/makefile-lint.yml: - simplify yaml-linting in makeile-lint.yml
Changes in file .github/workflows/markdown-lint.yml: - back to maintainable state hopefully 🙈
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
.github/workflows/makefile-lint.yml (1)
105-118: Consider consolidating duplicate print_error functions.The
print_errorfunction is duplicated. Consider moving it to a shared location or defining it once at the workflow level.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/makefile-lint.yml(4 hunks).github/workflows/markdown-lint.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/makefile-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
.github/workflows/markdown-lint.yml (1)
Pattern .github/**: * When the project is hosted on GitHub: All GitHub-specific configurations, templates, and tools should be found in the '.github' directory tree.
- 'actionlint' erroneously generates false positives when dealing with GitHub's
${{ ... }}syntax in conditionals. - 'actionlint' erroneously generates incorrect solutions when suggesting the removal of valid
${{ ... }}syntax.
📓 Learnings (3)
📓 Common learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers scripts to include explicit data processing steps that produce the expected output, ensuring accurate and useful results.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:50.461Z
Learning: For future scripts, reactive-firewall prefers to use only POSIX built-ins (like `printf` instead of `echo`) to align with CEP-8 conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-21T02:05:08.096Z
Learning: Reactive-firewall prefers to specify the full raw URL when fetching files from gists to ensure correct retrieval of content, particularly when the file is in markdown format.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T04:45:25.464Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, avoiding raw source code dumps, and focusing on clear explanations and real-world examples.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-12T10:29:55.563Z
Learning: For future scripts, reactive-firewall prefers to keep the shebang line with an extra space (e.g., `#! /bin/bash`) as per project style.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: LICENSE.md:36-43
Timestamp: 2024-11-14T07:03:35.207Z
Learning: Reactive-firewall prefers that I verify URLs before suggesting changes, and avoid raising hypothetical issues without verification.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-12-02T08:20:59.928Z
Learning: When implementing package.json, reactive-firewall prefers to maintain flexibility in npm installation methods, supporting both global (-g) installations and local project-specific installations.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#0
File: :0-0
Timestamp: 2024-11-29T01:10:18.136Z
Learning: For future reviews, remember that reactive-firewall prefers documentation to be presented in a tone appropriate for developers, following the project's style and conventions.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#184
File: tests/check_pip:259-265
Timestamp: 2024-11-15T19:32:16.878Z
Learning: Reactive-firewall prefers that code changes adhere to SOLID design principles, specifically avoiding unnecessary complexity (AHA principle), maintaining the Single Responsibility Principle, and ensuring complex work is done on separate development branches (Open/Closed Principle).
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#110
File: docs/USAGE.md:66-66
Timestamp: 2024-11-12T10:29:55.563Z
Learning: Reactive-firewall prefers to use "Caveat" as a heading instead of "Caution".
.github/workflows/makefile-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:34-39
Timestamp: 2024-11-29T09:46:47.010Z
Learning: For future scripts in workflow files (e.g., `.github/workflows/makefile-lint.yml`), reactive-firewall prefers concise code and avoids unnecessary variable assignments when possible, such as using `${{ vars.YAML_ARGS }}` directly without assigning it to another variable.
.github/workflows/markdown-lint.yml (3)
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/makefile-lint.yml:39-39
Timestamp: 2024-11-29T10:13:46.501Z
Learning: For future reviews, remember that reactive-firewall may handle regressions caused by `${YAML_ARGS}` being incorrectly set, and prefers that suggestions consider potential issues with this variable.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:37-37
Timestamp: 2024-12-03T03:18:50.682Z
Learning: In `.github/workflows/markdown-lint.yml`, reactive-firewall intentionally uses the regex pattern `if [[ "${YAML_ARGS}" =~ ^*[!a-zA-Z0-9\ \-\.]*$ ]]; then` for validation and prefers to keep it as is.
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
🪛 actionlint (1.7.4)
.github/workflows/makefile-lint.yml
57-57: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/markdown-lint.yml
43-43: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
51-51: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (7)
.github/workflows/markdown-lint.yml (4)
28-35: LGTM! Well-structured error handling function.
The print_error function follows good practices:
- Uses POSIX-compliant
printf - Provides clear error message formatting
- Includes file and line information for better debugging
36-37: LGTM! Proper error handling for installations.
The error handling is well-implemented:
- Uses the
print_errorfunction consistently - Provides clear error messages
- Uses appropriate exit codes
48-53:
Quote YAML_ARGS to prevent word splitting.
The yamllint command has unquoted arguments which could lead to word splitting.
Apply this diff to fix the issue:
- yamllint ${YAML_ARGS} .markdownlint.yaml
+ yamllint "${YAML_ARGS}" .markdownlint.yaml🧰 Tools
🪛 actionlint (1.7.4)
51-51: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
40-45:
Quote YAML_ARGS to prevent word splitting.
The yamllint command has unquoted arguments which could lead to word splitting.
Apply this diff to fix the issue:
- yamllint ${YAML_ARGS} .github/workflows/markdown-lint.yml
+ yamllint "${YAML_ARGS}" .github/workflows/markdown-lint.yml⛔ Skipped due to learnings
Learnt from: reactive-firewall
PR: reactive-firewall/multicast#227
File: .github/workflows/markdown-lint.yml:36-48
Timestamp: 2024-11-29T10:13:52.507Z
Learning: For future reviews, remember that in `.github/workflows/markdown-lint.yml`, issues may be caused by `${YAML_ARGS}` being incorrectly set, leading to regressions that require adjustments beyond quoting or validation logic.
🧰 Tools
🪛 actionlint (1.7.4)
43-43: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/makefile-lint.yml (3)
3-16: LGTM! Well-documented function.
The documentation follows CEP-7 guidelines and provides clear information about:
- Function purpose
- Arguments and their usage
- Output format
42-52: LGTM! Consistent error handling implementation.
The print_error function is well-implemented:
- Follows the documented interface
- Uses POSIX-compliant commands
- Provides appropriate error handling for each tool
55-59:
Quote YAML_ARGS to prevent word splitting.
The yamllint command has unquoted arguments which could lead to word splitting.
Apply this diff to fix the issue:
- yamllint ${YAML_ARGS} .github/workflows/makefile-lint.yml
+ yamllint "${YAML_ARGS}" .github/workflows/makefile-lint.yml🧰 Tools
🪛 actionlint (1.7.4)
57-57: shellcheck reported issue in this script: SC2086:info:2:10: Double quote to prevent globbing and word splitting
(shellcheck)
Changes in file .github/workflows/makefile-lint.yml:
Logs
Changes in file .github/workflows/markdown-lint.yml:
Logs
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
New Features
YAML_ARGSto ensure only valid characters are used.