-
Notifications
You must be signed in to change notification settings - Fork 144
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
Add sphinx-builder linkcheck
#2489
Conversation
Related to #2489. This PR does not fix all invalid links, but the rest should be caused by incorrect handling of links in a template when rendering them; these should be resolved by the work related to #2868. Co-authored-by: Miroslav Vadkerti <[email protected]>
Related to #2489. This PR does not fix all invalid links, but the rest should be caused by incorrect handling of links in a template when rendering them; these should be resolved by the work related to #2868. Co-authored-by: Miroslav Vadkerti <[email protected]>
e511083
to
779a9fc
Compare
@happz looks like we have a few more broken links being generated. Looking into how it's being generated. |
Oh wow, this is what it expanded for me: * Verified by `/tests/core/adjust <https://github.com/LecrisUT/tmt/tree/feat/linkcheck/tests/core/adjust/main.fmf>`_ It seems the generation automatically puts in the git repo and branch, but then on a PR, the git repo can be either target or source depending on the checkout. I think that is where it trips up 🤔 |
Yes, my #2940 did not try to address everything. There's #2868 for the links to tmt internals and tests as created by the template, which seems to be the vast majority of the remaining broken links.stories
Check out #2868 (comment). I'd be happy to look into fixing this, since it's the code I wrote and left in less than ideal state, but if you feel like diving in, be my guest :) Today I'd go with the idea mentioned in the last paragraph, i.e. moving the macro away from the template, making it a full-fledged Python code & expose it back to the template, mostly because it can be tested easily. I'd add a couple of unit tests to verify it's rendering links correctly - should be mostly
That's one issue for sure, there are more though, e.g. some links lead to |
Thanks @happz!
I'll be afk for the rest of this week unfortunately. Also, I've familiarized myself with the templates a bit and think I get the general idea, but there is still plenty of things I'm seeing for the first time to catch-up with. |
With #3001 applied, I get no broken links reported from the linkcheck builder. |
Let me rebase on your commit first just to make sure it works for forks as well |
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.
LGTM
/packit build |
741d145
to
529a750
Compare
529a750
to
67e519f
Compare
/packit build |
Hmm, blocked by #3108 :/ |
Or maybe superseded-by since that one contains the commit from here |
Let's keep the |
Signed-off-by: Cristian Le <[email protected]>
67e519f
to
3139f93
Compare
/packit build |
The extended unit tests failure will be fixed by #3227. Remaining failure is irrelevant. |
Related to teemtee#2489. This PR does not fix all invalid links, but the rest should be caused by incorrect handling of links in a template when rendering them; these should be resolved by the work related to teemtee#2868. Co-authored-by: Miroslav Vadkerti <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Depends on: #2483
There are a bunch of linkcheck failures, many of which come from the auto-generated pages. Not sure how these should be handled