-
Notifications
You must be signed in to change notification settings - Fork 242
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
[MEI-23]: Broken links checker #2507
Conversation
475eca3
to
2f56df8
Compare
Hi there 👋 It looks like this PR introduces broken links to the docs, please take a moment to fix them before merging:
Thank you 🙏 |
9e278c4
to
98f01d2
Compare
1fd2326
to
6a83b61
Compare
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.
Hum ok, I understand I guess, the line is about the github action itself and the second line is to block the PR for the merge because a comment has been generated |
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.
Ok for me 👌
I don't approve yet because I think we should fix the dead link before continuing.
I would like to see everything works when there are no dead links.
Do we plan to do another PR to fix the dead links or will it be done in this PR?
I think we can create a new PR to fix the links, but I don't have the context to know where they should point to CC @guimachiavelli |
yeap, it's in my list, just haven't gotten around to it yet as I'm focussing on getting everything ready for v1.3. I should have some time on thursday, if all goes according to plan. |
Hmmm…I just merged a fix into this branch, but link validation failed again. It seems none of the broken links in the new report had been previously detected (I made a copy of the original report on #2521). A separate concern: would it be possible to create an npm/yarn task to run the checker locally? |
You're absolutely right, I'll try to recreate each step to find out why they were skipped the first time.
It's possible, I should readapt the checker a bit to skip the github action specific sections if it's called locally. |
Hey, looks like a broken link has been seen by a user despite @guimachiavelli fixed all the links reported by the checker in #2524 |
That's expected because #2524 was never merged into I don't know if its desirable, but do you have permission to merge this PR despite the validation failing @curquiza? I don't have much time today to review the issue with the new links reported (#2507 (comment)). I think it may be the best for the users to have this merged and I'll open a new PR with the fixes later. |
Oh yes you are right, my bad, I'm tired 😅 The best would have been to merge it on Knowing this, you are right, let's merge this. It will remove the broken links from documentation. I will temporarily change the settings of the repo to merge despite the failure. I will inform gui about this so that he's aware of it when coming back from holiday |
Pull Request
This PR adds a new action to check if any broken links are introduced before merging to
main
.Related issue
MEI-23