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

common: Use ifconfig directive in place of [site wiki=...] #4438

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TunaLobster
Copy link
Contributor

@TunaLobster TunaLobster commented Jul 12, 2022

This one might be interesting from a build perspective. In the conf.py files there is an extension for the ifconfig directive that has been there for 6 years. The extension allows for controlling the contents of the page based on a Python style expression and properties from conf.py. In each of the conf.py files there is the project property which is the name of the site. That makes it pretty useful to use in the ifconfig directive. There are 232 uses of [site wiki=...] in the common directory right now and I expect all of them could be replaced by this directive. It should speed up building some, but I haven't checked how much yet. Should also make incremental builds easier to attain.

https://www.sphinx-doc.org/en/master/usage/extensions/ifconfig.html

I tried searching if this had been brought up before or what Hamish might have intended with include the extension, but nothing turned up.

@TunaLobster TunaLobster marked this pull request as draft July 12, 2022 19:47
@TunaLobster
Copy link
Contributor Author

Old on the left and new on right. Plane on top and Copter on the bottom. I don't know why bullet points are not showing up in my build, but it is not related to this change.

image

@Hwurzburg
Copy link
Contributor

my issue with using this is the lack of clarity of where the section ends covered by the ifconfig.....like the problem of using blocks....the current way is clear and easy to use....this is kinda if it aint broke dont fix it....and mixing the two would be confusing to new contributors...unless there is a clear advantage to this not mentioned above....build time is not long at the moment and I doubt this halves it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants