-
Notifications
You must be signed in to change notification settings - Fork 368
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
docs/ci: use myst, fix broken links, add linkcheck test, remove deprecated distutils, avoid 2x job triggers #511
docs/ci: use myst, fix broken links, add linkcheck test, remove deprecated distutils, avoid 2x job triggers #511
Conversation
Additional notes, that seem quite outdated at the time of writing May 2022, | ||
are available about authorizing users part of specific Google Groups are | ||
:ref:`available here <google-groups-label>`. Contributions to update these | ||
and re-verify this functionality are most welcome. | ||
|
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.
Oh yes! 👀
.github/workflows/test.yml
Outdated
paths-ignore: | ||
- "docs/**" | ||
- ".github/workflows/*.yaml" | ||
- "!.github/workflows/publish.yaml" |
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.
Any reason not to ignore the publish workflow here?
- "!.github/workflows/publish.yaml" | |
- "!.github/workflows/test.yaml" |
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.
Nice catch! It was copy pasted from a workflow named publish, but I had forgo to update it to de-ignore the test.yaml
workflow. The idea was that "ignore all workflows (line before), but not this workflow"
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.
Extremely grateful for this PR @consideRatio! I believe we should merge it!
Co-authored-by: Georgiana Elena <georgiana.dolocan@gmail.com>
@GeorgianaElena thank you for the review!!! I applied your suggestions (tweaked, see #511 (comment)), I'll go for a merge right now then! |
The docs had not been updated for a long time, so this PR is a significant overhaul. There were plenty of broken links etc.
Closes #480.
There is one thing that doesn't fit so well in this PR, which is 72ce042 that was added because I could not get
make html
ormake linkcheck
to function properly without making some changes there. Closes #505 which was a dedicated PR trying to do something about that as well.