-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
MkDocs projects use RTD's analytics privacy improvements #4013
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -100,18 +100,23 @@ def append_conf(self, **__): | |
if 'theme_dir' not in user_config and self.use_theme: | ||
user_config['theme_dir'] = TEMPLATE_DIR | ||
|
||
yaml.safe_dump( | ||
user_config, | ||
open(os.path.join(self.root_path, 'mkdocs.yml'), 'w') | ||
) | ||
|
||
docs_path = os.path.join(self.root_path, docs_dir) | ||
|
||
# RTD javascript writing | ||
rtd_data = self.generate_rtd_data(docs_dir=docs_dir, mkdocs_config=user_config) | ||
with open(os.path.join(docs_path, 'readthedocs-data.js'), 'w') as f: | ||
f.write(rtd_data) | ||
|
||
# Use Read the Docs' analytics setup rather than mkdocs' | ||
# This supports using RTD's privacy improvements around analytics | ||
user_config['google_analytics'] = None | ||
|
||
# Write the mkdocs configuration | ||
yaml.safe_dump( | ||
user_config, | ||
open(os.path.join(self.root_path, 'mkdocs.yml'), 'w') | ||
) | ||
|
||
def generate_rtd_data(self, docs_dir, mkdocs_config): | ||
"""Generate template properties and render readthedocs-data.js.""" | ||
# Get the theme name | ||
|
@@ -120,6 +125,12 @@ def generate_rtd_data(self, docs_dir, mkdocs_config): | |
if theme_dir: | ||
theme_name = theme_dir.rstrip('/').split('/')[-1] | ||
|
||
# Use the analytics code from mkdocs.yml if it isn't set already by Read the Docs | ||
analytics_code = self.version.project.analytics_code | ||
if not analytics_code and mkdocs_config.get('google_analytics'): | ||
# http://www.mkdocs.org/user-guide/configuration/#google_analytics | ||
analytics_code = mkdocs_config['google_analytics'][0] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this always a list? Feels a bit brittle and might just grab one character if it's just a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docs link leads me to believe it is a list. I'm fine being a bit more defensive here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, it looks like that is probably unnecessary: https://github.com/mkdocs/mkdocs/blob/71ebf35/mkdocs/config/defaults.py#L51-L53 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is guaranteed to be either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
# Will be available in the JavaScript as READTHEDOCS_DATA. | ||
readthedocs_data = { | ||
'project': self.version.project.slug, | ||
|
@@ -134,7 +145,7 @@ def generate_rtd_data(self, docs_dir, mkdocs_config): | |
'api_host': getattr(settings, 'PUBLIC_API_URL', 'https://readthedocs.org'), | ||
'commit': self.version.project.vcs_repo(self.version.slug).commit, | ||
'global_analytics_code': getattr(settings, 'GLOBAL_ANALYTICS_CODE', 'UA-17997319-1'), | ||
'user_analytics_code': self.version.project.analytics_code, | ||
'user_analytics_code': analytics_code, | ||
} | ||
data_json = json.dumps(readthedocs_data, indent=4) | ||
data_ctx = { | ||
|
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.
Should we delete this, instead of setting it to
None
?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.
I use
del
extremely rarely, but I don't have a particularly strong opinion.