-
Notifications
You must be signed in to change notification settings - Fork 16
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
Added checking markdown links #81
Conversation
@akihironitta I guess I need some approval to run workflows. |
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.
@akihironitta I think we shall describe in the readme that check-
are meant to be a re-usable workflow and ci-
to run for the particular repo
I think I need to update the config json to allow for skipping CHANGELOG links. |
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.
Thanks! LGTM! In this PR or in a follow-up PR, could you call this reusable workflow from ci-use-checks.yml
? This way, we know the new workflow actually works :)
It seems like one of the links returns 404. Mind resolving the error in this PR? Just marked this PR as draft so that we don't accidentally merge this PR into master. Once it's fixed, please mark this PR as ready! |
So it fails in the main README where the docs is currently showing unknown. The link attached is, |
@shenoynikhil Oh sorry this is the correct one! https://lightning-devtools.readthedocs.io/en/latest/ @Borda We might want to update the domain to something like |
Should I use the one you have shared for now? |
I updated the README with the link you shared. I guess whenever the domain changes, the README.md will also change accordingly. |
@shenoynikhil Thanks! Overall looks good but looking at it again, it has to be a reusable workflow. (ref) Examples we have are named |
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.
Requesting changes just for me to remind myself of coming back to this PR :)
Thank you so much! |
|
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.
Sorry for the delay! Thank you for working on this :)
hi @akihironitta @shenoynikhil what is missing to land this? :) |
Hi @shenoynikhil! Mind having a look at my comment above? If you're too busy to finish this, I can contribute to this by directly pushing commits to your branch as a weekend project (if you're fine with it) 🚀 |
.github/workflows/check-md-links.yml
Outdated
- uses: gaurav-nelson/github-action-markdown-link-check@v1 | ||
with: | ||
use-quiet-mode: 'yes' | ||
config-file: '.github/workflows/markdown.links.config.json' |
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.
if we g with workflow_call
then this shall be input argument
also I would consider to make this file on the fly (dump to the CI run) if none is provided 🐿️
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.
That makes sense, did the first part. Will have to think about how to do the second bit. Based of initial ideas (ignore my github workflow building skills), would the jobs part of the workflow look like this,
jobs:
markdown-link-check:
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@master
- name: Check for config file, if not create it
run: |
if [ ! -f ${{ inputs.config-file }} ]; then
echo "config.json not found, creating one..."
echo '{
"ignorePatterns": [
{
"pattern": "^https://github.com/Lightning-AI/lightning/pull/.*"
}
],
"httpHeaders": [
{
"urls": ["https://github.com/", "https://guides.github.com/", "https://help.github.com/", "https://docs.github.com/"],
"headers": {
"Accept-Encoding": "zstd, br, gzip, deflate"
}
}
]
}' > ${{ inputs.config-file }}
else
echo "config.json found, no need to create one."
fi
- uses: gaurav-nelson/github-action-markdown-link-check@v1
with:
use-quiet-mode: 'yes'
config-file: ${{ inputs.config-file }}
656bdff
to
11958a8
Compare
@akihironitta ^^ 🐿️ |
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Before submitting
Based on the Lightning-AI/pytorch-lightning#16168, added workflow files for checking markdown links.
What does this PR do?
Fixes #80.
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃