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

Single analytics file for all builders #3896



Copy link

@davidfischer davidfischer commented Apr 3, 2018

This pull request breaks out the analytics file readthedocs-dynamic-include.js which is typically created with each docs build into a separate file which will live at

  • This PR only removes readthedocs-dynamic-include.js only for mkdocs. Referencing the same analytics file will be done for sphinx builds as well but that change will be in (probably an update on readthedocs/readthedocs-sphinx-ext#33)
  • This PR sets the groundwork for both mkdocs and sphinx to use the same analytics file rather than having any change to analytics require simultaneous corresponding changes across multiple repos.
  • If we ever swap out Google Analytics, that could be done server side and we would immediately see analytics with an alternative system rather than waiting for projects to be rebuilt.
  • This PR also puts some of my (our?) thoughts on Google Analytics to paper as this isn't an uncommon question we get from privacy conscious users.
  • This will result in a minor performance improvement because multiple project docs will use the same readthedocs-analytics.js file which is cached (albeit only for a week with our current settings).
Copy link

@ericholscher ericholscher left a comment

Think this is a great direction. Having the ability to change this stuff dynamically in the future is a good first step towards another approach when we choose to move forward with something else.


// Read the Docs instructs Google to anonymize IPs sent to them before they are stored (see below).

// We are always exploring our options with respect to analytics and if you would like
// to discuss further, feel free to open an issue on github.
Copy link

@ericholscher ericholscher Apr 4, 2018

Choose a reason for hiding this comment

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



@ericholscher ericholscher merged commit c83b17a into readthedocs:master Apr 5, 2018
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants