-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove separate script for nightly devsite diff #4105
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
Conversation
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.
A nit and a clarifying question, otherwise LGTM.
"The url prefix for links to code.", | ||
) | ||
|
||
flags.DEFINE_bool("search_hints", True, "Include metadata search hints in the generated files") | ||
|
||
flags.DEFINE_string("site_path", "quark/cirq/api_docs/python", "Path prefix in the _toc.yaml") | ||
flags.DEFINE_string("site_path", "reference/python", "Path prefix in the _toc.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.
What prompted this change? At first glance it doesn't seem related to either of the linked issues.
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 non-material, because the internal caller overrides this anyway with the correct string, but it was an outdated default value (we had this URL structure when we first had the internal staging site hosted on tensorflow's site) that I just updated. I'll include that in the PR description.
After the v0.11 release there is no need for a separate script for the extra modules. I also added the fix for the code_prefix_url which should fix the issue with the broken "View Source" functionality. An unrelated nit that I changed is the default value for the site_url parameter - we had this URL structure when we first had the internal staging site hosted on tensorflow's site, now it's in line with what we have live. For the next module extractions this script will need to cater for both the stable (v0.11) and the nightly (from master) version, it will be more manageable that way. Fixes quantumlib#4066. Fixes quantumlib#4104.
After the v0.11 release there is no need for a separate script for the extra modules.
I also added the fix for the code_prefix_url which should fix the issue with the broken "View Source" functionality.
An unrelated nit that I changed is the default value for the site_url parameter - we had this URL structure when we first had the internal staging site hosted on tensorflow's site, now it's in line with what we have live.
For the next module extractions this script will need to cater for both the stable (v0.11) and the nightly (from master) version, it will be more manageable that way.
Fixes #4066.
Fixes #4104.