-
Notifications
You must be signed in to change notification settings - Fork 0
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
Feat/improve doc build logic #502
base: main
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
=======================================
Coverage 92.45% 92.45%
=======================================
Files 64 64
Lines 2599 2599
=======================================
Hits 2403 2403
Misses 196 196 ☔ View full report in Codecov by Sentry. |
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.
Thanks, @moe-ad. Just left some minor comments.
doc/source/conf.py
Outdated
DOWNLOAD_FILES_DIR.mkdir(exist_ok=True) | ||
|
||
for file_path in ZIPPED_FILES_DIR.glob("*"): | ||
EXAMPLES = { |
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.
why using all caps for variables like examples, source_dir etc?
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.
This is because those more or less do not change, like constants.
If you prefer, I can move examples to the top of the file and pass it as an argument to the function instead.
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.
SOURCE_DIR
should just be a lower case variable, it depends on the input app
, it's not a constant.
Regarding EXAMPLES
, either you move it outside of the function as a constant or you make it lower case inside the function. Either is fine with me.
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.
Thanks! Implemented your suggestions.
a0d4a66
to
fef5eb5
Compare
Description
This PR will close #501. Pre-build scripts are set up as hooks in conf.py and "sphinx-build ..." is being called directly in tox instead of "make ..."
Checklist