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

APIv3: add analytics field to Project object #10239

Closed
wants to merge 2 commits into from

Conversation

humitos
Copy link
Member

@humitos humitos commented Apr 12, 2023

New analytics field that exposes:

  • the Google code if the project has it configured
  • whether or not the global Google/Plausible Read the Docs analytics is enabled

This field is going to be useful for the new "Analytics Addon".

Related #10216

New `analytics` field that exposes:

- the Google code if the project has it configured
- whether or not the global Google/Plausible Read the Docs analytics is enabled

This field is going to be useful for the new "Analytics Addon".

Related #10216
@humitos humitos requested a review from a team as a code owner April 12, 2023 08:57
@humitos humitos requested a review from stsewd April 12, 2023 08:57
@humitos humitos requested a review from a team as a code owner April 12, 2023 09:01
@humitos humitos requested a review from benjaoming April 12, 2023 09:01
@humitos
Copy link
Member Author

humitos commented Apr 13, 2023

We are deprecating the GA analytics in #9530, so may not make sense to expose this here?

@benjaoming
Copy link
Contributor

Yeah, given the path laid out in #9530, it doesn't make sense to expose the field so some new integration would end up using it.

Are you okay with closing this @humitos ?

@humitos
Copy link
Member Author

humitos commented May 1, 2023

@benjaoming

whether or not the global Google/Plausible Read the Docs analytics is enabled

We need to decide what to do with this field, tho. We have ~1900 projects disabling Read the Docs analytics. I think I'm fine removing this config and always enforce our own analytics to projects, https://docs.readthedocs.io/en/stable/analytics.html#disabling-google-analytics-on-your-project -- I'm not sure if it adds too much value to our platform 🤷🏼

Copy link
Contributor

@benjaoming benjaoming left a comment

Choose a reason for hiding this comment

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

The change is pretty straightforward. And the dictionary structure with google and readthedocs is great!

There are some weird test failures, but maybe merging main can fix that?

@benjaoming
Copy link
Contributor

@humitos You mean that it's better to expose it in the API and take a separate decision about deprecating / keeping it? I think it's fine - actually, it's not bad that someone would want to use this part of the API, just means that it's relevant.

@humitos humitos added the Status: blocked Issue is blocked on another issue label May 1, 2023
@humitos
Copy link
Member Author

humitos commented May 1, 2023

I'm saying we should not merge this PR yet. We need to define how people will/if decide whether or not they want us to include our own analytics in their docs.

@benjaoming
Copy link
Contributor

Okay, I'm confused about how a change that exposes a field in the API contributes to an actual change in configuration and documentation rendering.

@humitos humitos marked this pull request as draft May 24, 2023 09:18
@humitos
Copy link
Member Author

humitos commented Jun 30, 2023

I'm closing this PR because we are moving forward with the plan from #9530 (comment)

@humitos humitos closed this Jun 30, 2023
@stsewd stsewd deleted the humitos/api-projects-analytics branch June 30, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: blocked Issue is blocked on another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants