Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

458: Markdown linter as a local npm script & Git Action execution #503

Merged
merged 16 commits into from
Oct 8, 2024

Conversation

adigidh
Copy link

@adigidh adigidh commented Sep 24, 2024

fixes #458

Summary of Changes:

  • Custom Markdown Linting configuration : .markdownlint.yml that holds the rules (can be updated based on our requirements)
  • NPM script added to the project to complete markdown linting locally: markdownlint-cli2 is the CLI tool that helps us with the script execution.
  • npm run lint will produce results for the markdown linting errors.
  • npm run lint: fix will help to auto fix these issues for markdown linting
  • Updated Markdown files with corresponding linting fixes
  • Markdown Linter named Github Action added to the workflows which does the following:
    - Uses a custom configuration file
    - Runs markdown linter on markdown files in the docs directory
    - continue-on-error: true parameter set currently to not fail the github action on every pull request

@maia-iyer maia-iyer changed the base branch from dev to main September 25, 2024 15:09
Copy link
Collaborator

@mamy-CS mamy-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this, let's fix some of the lint errors on the pr docs/blogs.md and this should be good to go.

@adigidh
Copy link
Author

adigidh commented Sep 30, 2024

Thank you for this, let's fix some of the lint errors on the pr docs/blogs.md and this should be good to go.

Thanks for taking a look @mamy-CS.
Made the corresponding updates. Changed some yaml rules to avoid noise in the resulting markdown files.

@mamy-CS
Copy link
Collaborator

mamy-CS commented Oct 1, 2024

@adigidh we have a building failure in the frontend please check. https://github.com/spiffe/tornjak/actions/runs/11113268459/job/30918941927?pr=503

@mamy-CS
Copy link
Collaborator

mamy-CS commented Oct 2, 2024

@adigidh try the following steps to fix the npm authentication error:

  • Delete the package-lock.json file
  • Delete your local node_modules folder
  • Upgrade the node version to the latest
  • Update npm as well
  • Run npm install

@adigidh
Copy link
Author

adigidh commented Oct 2, 2024

@mamy-CS :
Looks like the project required node 18 and above, I'm already on the recent node versions..

node - v

v22.2.0

npm -v

10.7.0

I think I figured out the issue. Because the project by itself doesn't have a .npmrc file at the root of the frontend folder, when I do a npm install it tries to use the creds in my global ~/.npmrc file. Thereby the authentication issues as it tried to look to download these packages from internal artifactory. This can be solved by adding a .npmrc. Will make these changes and push shortly

@adigidh adigidh requested a review from mamy-CS October 4, 2024 13:39
Copy link
Collaborator

@mamy-CS mamy-CS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the pr @adigidh
/lgtm

@mamy-CS mamy-CS merged commit 12fed8d into spiffe:main Oct 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a markdown linter
2 participants