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

feat: add analytics #819

Merged
merged 6 commits into from
Jun 13, 2019
Merged

feat: add analytics #819

merged 6 commits into from
Jun 13, 2019

Conversation

10ko
Copy link
Member

@10ko 10ko commented Jun 5, 2019

No description provided.

@10ko 10ko force-pushed the feat-add-analytics branch 3 times, most recently from c4b2c7f to 45016f4 Compare June 12, 2019 22:08
@10ko 10ko marked this pull request as ready for review June 12, 2019 22:17
@eysi09 eysi09 requested a review from edvald June 13, 2019 08:07
@eysi09
Copy link
Collaborator

eysi09 commented Jun 13, 2019

@edvald I requested you as a reviewer since you have more experience with Amplitude and these things in general.

docs/reference/commands.md Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/config-store.ts Show resolved Hide resolved
garden-service/src/config-store.ts Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
@10ko 10ko force-pushed the feat-add-analytics branch 5 times, most recently from cdc32e3 to bdd6eb7 Compare June 13, 2019 16:16
@10ko
Copy link
Member Author

10ko commented Jun 13, 2019

Thanks @edvald, addressed all your comments and refactored quite a bit.
Also added a new Api Key for "prod".

✌️

edvald
edvald previously requested changes Jun 13, 2019
Copy link
Collaborator

@edvald edvald left a comment

Choose a reason for hiding this comment

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

Mostly minor comments and doc issues, otherwise 👍👍

docs/reference/commands.md Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
garden-service/src/analytics/analytics.ts Outdated Show resolved Hide resolved
10ko added 4 commits June 13, 2019 20:41
This feature add the Analytics class to manage opt-in/opt-out and
tracking of events happening within the CLI and the Dashboard.
Introduces also the command `garden update-analytics` to update your
preferences.
@edvald edvald dismissed their stale review June 13, 2019 19:15

Needs another pair of eyes

To help us make Garden better you can opt in to the collection of usage data.
We make sure all the data collected is anonymized and stripped of sensitive
information. We collect data about which commands are run, what tasks they trigger,
the API calls are made to your local Garden server, as well as some info
Copy link
Collaborator

Choose a reason for hiding this comment

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

"That" (or similar) missing in: "the API calls are made to your local..."

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also would've put a comma in: "To help us make Garden better, you can opt in to the collection of usage data." After "better" that is. But I guess that's a style preference.

To help us make Garden better you can opt in to the collection of usage data.
We make sure all the data collected is anonymized and stripped of sensitive
information. We collect data about which commands are run, what tasks they trigger,
the API calls are made to your local Garden server, as well as some info
Copy link
Collaborator

Choose a reason for hiding this comment

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

I also would've put a comma in: "To help us make Garden better, you can opt in to the collection of usage data." After "better" that is. But I guess that's a style preference.

@edvald edvald force-pushed the feat-add-analytics branch 3 times, most recently from 44f53ae to ec9f7b1 Compare June 13, 2019 19:33
@edvald edvald merged commit a2fa49e into master Jun 13, 2019
@edvald edvald deleted the feat-add-analytics branch June 13, 2019 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants