HDDS-14546. [Website v2] Add lint and lint:fix commands#318
HDDS-14546. [Website v2] Add lint and lint:fix commands#318errose28 merged 9 commits intoapache:masterfrom
lint and lint:fix commands#318Conversation
…re launching it (apache#326)" This reverts commit 20b71d0. Apply this commit to the master branch, not the HDDS-9225-website-v2 branch. We could still use this branch for future staging development
|
PSA: Please switch PR target branch to |
|
@smengcl can I have a review please? |
There was a problem hiding this comment.
I don't think the changes in this file of this PR is valid? Those are the remainder from the v2 launch.
There was a problem hiding this comment.
Agree, the new site is live. We should use the master branch's version.
There was a problem hiding this comment.
Pull request overview
This pull request adds linting capabilities to the Apache Ozone website v2 project by introducing lint and lint:fix npm scripts that run markdownlint and yamllint checks. The PR consolidates the previous separate GitHub Actions workflows for markdown and YAML linting into a single unified lint job, removes the now-redundant shell scripts, and fixes markdown heading level issues discovered by markdownlint in the release notes files. Additionally, it updates the docusaurus configuration with TODO comments and temporary URLs for the staging environment.
Changes:
- Added
lintandlint:fixcommands to package.json for running markdownlint and yamllint - Consolidated GitHub Actions workflows by merging
lint-markdownandlint-yamljobs into a singlelintjob - Fixed markdown heading hierarchy in release notes to comply with markdownlint rules and Keep a Changelog format
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added lint and lint:fix scripts for running markdownlint and yamllint |
| .github/workflows/static.yml | Consolidated lint-markdown and lint-yaml jobs into single lint job |
| .github/scripts/markdownlint.sh | Removed - functionality moved to package.json script |
| .github/scripts/yamllint.sh | Removed - functionality moved to package.json script |
| src/pages/release-notes/2.1.0.md | Fixed heading levels to follow proper hierarchy (h3→h4 for changelog subsections, h2→h3 for Downloads/Documentation) |
| src/pages/release-notes/2.0.0.md | Fixed heading levels to follow proper hierarchy |
| docusaurus.config.js | Added TODO comments and updated URLs/regex for staging environment |
| CONTRIBUTING.md | Documented new lint commands and their usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
package.json
Outdated
| "write-heading-ids": "docusaurus write-heading-ids" | ||
| "write-heading-ids": "docusaurus write-heading-ids", | ||
| "lint": "markdownlint . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"", | ||
| "lint:fix": " markdownlint --fix . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"" |
There was a problem hiding this comment.
Extra leading space in the lint:fix script value. The value should start with "markdownlint" not " markdownlint".
| "lint:fix": " markdownlint --fix . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"" | |
| "lint:fix": "markdownlint --fix . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"" |
package.json
Outdated
| "lint": "markdownlint . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"", | ||
| "lint:fix": " markdownlint --fix . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"" |
There was a problem hiding this comment.
The yamllint command is used in both lint scripts but yamllint is not listed as a dependency in package.json. While yamllint is pre-installed on GitHub Actions runners, developers running these commands locally will need to install yamllint separately via pip or their package manager. Consider adding a note in the CONTRIBUTING.md file about this prerequisite, or explore using a Node.js-based YAML linter that can be added as a devDependency.
There was a problem hiding this comment.
While yamllint is pre-installed on GitHub Actions runners, developers running these commands locally will need to install yamllint separately via pip or their package manager. Consider adding a note in the CONTRIBUTING.md file about this prerequisite
@yuriipalam can you add this to the contributing guide?
|
@yuriipalam Could you please fix the config.js file and resolve the conflicts? |
|
Sure @ptlrs |
errose28
left a comment
There was a problem hiding this comment.
Thanks for adding this @yuriipalam. Can you address the open comments from the AI as well, they look valid.
I think this was broken before this PR, but while we are making changes in this area we should add build to the ignore list in the yamllint config. This passes in CI because lint is run before build, but it will fail for developers locally if they have an old build and then try to lint their current changes before building again.
package.json
Outdated
| "lint": "markdownlint . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"", | ||
| "lint:fix": " markdownlint --fix . && yamllint --format=colored \"$(git rev-parse --show-toplevel)\"" |
There was a problem hiding this comment.
While yamllint is pre-installed on GitHub Actions runners, developers running these commands locally will need to install yamllint separately via pip or their package manager. Consider adding a note in the CONTRIBUTING.md file about this prerequisite
@yuriipalam can you add this to the contributing guide?
|
Thanks for the review @errose28 |
|
We just need to close the loop on this markdown change but everything else looks good! |
errose28
left a comment
There was a problem hiding this comment.
Thanks for adding this @yuriipalam. I just added back the 2.0.0 release page from master as well to fix the same as 2.1.0. Once CI is green this should be ready to merge.
What changes were proposed in this pull request?
Add
lintandlint:fixcommands that runmarkdownlintandyamllintpackages.lintjust checks for errorslint:fixactually fixes issues which are possible to fix automatically. It doesn't work foryamllintthough, it can only check the files.In the next PR we plan to integrate the ESLint plugin.
There were some issues with the markdown files found by
markdownlint. I fixed them, but it has to be reviewed by someone since I'm not familiar with the product. Some things required manual fixing, like proper heading ordering.markdownlintprohibits bypassing heading levels, e.g., jump from an h2 to h4 without an h3 heading between the two.What is the link to the Apache Jira?
HDDS-14546
How was this patch tested?