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

Introducing ember-plausible #285

Merged
merged 6 commits into from
Jun 10, 2022
Merged

Conversation

benjay10
Copy link
Contributor

@benjay10 benjay10 commented Jun 9, 2022

This PR removes old analytics set-up based on Plausible directly and introduced the ember-plausible addon. Analytics are enabled on all routes by default.

“[BenJay10]” added 4 commits June 9, 2022 22:17
This uses the global configuration without any enabling or custom goals
in the application. This might change in the future when automatic
enabling is removed.
Using this hook, we can make sure the promise for enabling the analytics
is finished, before continuing with the rest of the route. Something we
cannot control in an initializer. The automatic enabling might be
removed in the future, so we moved the code.
@benjay10 benjay10 added the enhancement New feature or request label Jun 9, 2022
@benjay10 benjay10 marked this pull request as ready for review June 9, 2022 22:46
Copy link
Member

@nvdk nvdk left a comment

Choose a reason for hiding this comment

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

in the interest of backwards compatibility (and not having to change the env vars on all servers) I would keep using EMBER_ANALYTICS_APP_DOMAIN. If we then also provide a default for conf.apiHost in the application route no config changes need to happen on the server.

@benjay10
Copy link
Contributor Author

@nvdk Do you want to include some default URL for the analytics endpoint in the code/config?

@nvdk
Copy link
Member

nvdk commented Jun 10, 2022

@benjay10 sorry wasn't very clear on this, as I was unsure myself. I think given that's it's only on two instances it's ok. I'll just adjust the env vars accordingly.

@nvdk nvdk merged commit 7095ff6 into development Jun 10, 2022
@nvdk nvdk deleted the feature/introducing-plausible branch June 10, 2022 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants