Skip to content
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

Simplify the documentation building #2483

Merged
merged 6 commits into from
May 7, 2024
Merged

Conversation

LecrisUT
Copy link
Contributor

@LecrisUT LecrisUT commented Nov 17, 2023

I've added these as GitHub Actions instead of pre-commits because:

  • It usually requires the installation of tmt itself
  • It can be a bit heavy for pre-commits

docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@happz
Copy link
Collaborator

happz commented Nov 18, 2023

Another one considered was spelling, but that one would require to maintain a dictionary of technical words used. Can add it as well if you think it's useful.

I think it would be a nice addition. Or a similar tool. Having a spellchecker in play would be nice, to avoid typos and improve consistency (saying "html" at one place and "HTML" elsewhere..., or my classic "tmt" vs "Tmt" vs "TMT" :)

It is a bit annoying because it doesn't pick up on apostrophes (e.g. we've)

Ok, I'd vote to get rid of the apostrophes and be verbose, i.e. "we have" in the docs...

or dashes (e.g. omni-present).

That we could improve by adding words to a list of allowed ones.

If there is an alternative to that would love to hear.

I'm not sure whether there are alternatives. Maybe it's also possible to use some integration with tools capable of checking not just spelling but also semantics ("it's" vs "its") and punctuation, like Grammarly (or ChatGPT :) But starting with at least a spellchecker would improve things.

Edit: I'm playing a bit with the spelling builder, putting together a list of words to allow, and it doesn't look as bad as I expected. 80/20 in action, I guess, plus some really valid issues found, like "url" vs "URL", "yaml" where "YAML" should be, and some actual technical stuff like "httpd" or "beakerlib". I'll try to make a PR out of it, this looks like fun :)

@LecrisUT
Copy link
Contributor Author

Ok, I've switched it to the combined Makefile+native sphinx format, and opened issues to track the rest. I've also factored out the other parts to dedicated PRs

@LecrisUT LecrisUT changed the title ci: Add more sphinx builders ci: Simplify sphinx builder Nov 20, 2023
@happz happz added the documentation Improvements or additions to documentation label Dec 16, 2023
@happz happz modified the milestone: 1.31 Dec 16, 2023
@happz
Copy link
Collaborator

happz commented Dec 16, 2023

/packit build

@LecrisUT
Copy link
Contributor Author

LecrisUT commented Mar 4, 2024

@happz @lukaszachy This should also go into a milestone. Do we have anything more to discuss for this one?

@happz happz added this to the 1.33 milestone Mar 4, 2024
@happz
Copy link
Collaborator

happz commented May 3, 2024

Works for me. Needs a rebase, but my local make docs produced the expected stuff (I use these docs often, to both read about tmt and check how the docs are rendered).

Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for preparing this, overall looks good, added just a few comments/questions.

.github/workflows/doc-tests.yml Outdated Show resolved Hide resolved
docs/Makefile Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@psss psss requested review from happz and lukaszachy May 6, 2024 11:08
@psss
Copy link
Collaborator

psss commented May 6, 2024

/packit build

@LecrisUT LecrisUT force-pushed the feat/sphinx-builders branch 2 times, most recently from e1741b8 to 62c9ee5 Compare May 6, 2024 12:52
@psss
Copy link
Collaborator

psss commented May 6, 2024

/packit build

@LecrisUT
Copy link
Contributor Author

LecrisUT commented May 6, 2024

Btw, you can just edit a comment that has /packit build and that would also re-trigger CIs. I usually add/remove a space at the end. Although this would not create a notification to the author or other subscribers that you are re-triggering a build, so the utility is questionable.

@psss
Copy link
Collaborator

psss commented May 6, 2024

Btw, you can just edit a comment that has /packit build and that would also re-trigger CIs.

Nice. Thanks for the hint!

docs/Makefile Outdated Show resolved Hide resolved
@psss psss changed the title Simplify sphinx builder Simplify the documentation building May 6, 2024
LecrisUT and others added 6 commits May 6, 2024 21:03
- Call `make generate` inside conf.py
- Remove sphinx builder make targets

Signed-off-by: Cristian Le <[email protected]><[email protected]>
It is covered by github action

Signed-off-by: Cristian Le <[email protected]>
@psss psss added the ci | full test Pull request is ready for the full test execution label May 6, 2024
@psss
Copy link
Collaborator

psss commented May 6, 2024

/packit build

@psss psss self-assigned this May 7, 2024
@psss
Copy link
Collaborator

psss commented May 7, 2024

The two failures in /plans/provision/virtual are irrelevant --> merging.

@psss psss merged commit 2879190 into teemtee:main May 7, 2024
20 of 21 checks passed
@LecrisUT LecrisUT deleted the feat/sphinx-builders branch May 7, 2024 10:14
@LecrisUT
Copy link
Contributor Author

LecrisUT commented May 7, 2024

Btw, linkcheck builder is already implemented here and it is finding a few broken links

The-Mule pushed a commit to The-Mule/tmt that referenced this pull request Oct 14, 2024
* Simplify `docs/Makefile`
* Fix pre-commit push branch target
* Add doc-tests to GH-Actions
* Remove `/plans/install/docs`

Signed-off-by: Cristian Le <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci | full test Pull request is ready for the full test execution documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants