-
Notifications
You must be signed in to change notification settings - Fork 88
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
Setup markdown linter #356
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here is some initial feedback
config/.markdownlint.yaml
Outdated
MD052: true | ||
|
||
# MD053/link-image-reference-definitions - Link and image reference definitions should be needed | ||
MD053: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.github/workflows/ci.yaml
Outdated
if: steps.changed-files.outputs.files != '' | ||
with: | ||
config: config/.markdownlint.yaml | ||
globs: ${{ steps.changed-files.outputs.files }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you try on your forked repo to open a PR and have this linter run?
I assume also that when it gets run there will be some errors due to these style: "consistent"
restrictions. Ideally these get fixed in the same PR (or a PR right after) as adding the linter so that follow-up PRs do not fail for the wrong reasons.
name: Markdown Linter | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout EIP Repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: Checkout EIP Repository | |
- name: Checkout portal-network-specs Repository |
.github/workflows/ci.yaml
Outdated
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout EIP Repository | ||
uses: actions/checkout@47fbe2df0ad0e27efb67a70beac3555f192b062f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can just use actions/checkout@v4
here, not sure why EIPs repo uses that specific revision.
I did run it, though locally with act and there was no error. |
.github/workflows/ci.yaml
Outdated
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Lint | ||
uses: DavidAnson/markdownlint-cli2-action@f5cf187ef11bd3a68a127321b794aa252ff23019 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I use version here too instead of the revision?
The shown error is strange though, I would not expect the line length to be checked considering this snippet from the used config:
|
I think so too |
This PR implements a Markdown linter in the CI pipeline.
Changes Made
Closes #321