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

chore: 0.13 analytics improvements #4179

Merged
merged 11 commits into from
May 12, 2023
Merged

chore: 0.13 analytics improvements #4179

merged 11 commits into from
May 12, 2023

Conversation

mkhq
Copy link
Contributor

@mkhq mkhq commented May 5, 2023

What this PR does / why we need it:

Adapt the analytics to 0.13 by including project stats for action kinds.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@mkhq mkhq force-pushed the 0.13-analytics-improvements branch 3 times, most recently from d57d453 to a082b5c Compare May 10, 2023 14:18
@mkhq mkhq marked this pull request as ready for review May 10, 2023 14:20
@mkhq mkhq requested a review from eysi09 May 10, 2023 14:23
@mkhq mkhq force-pushed the 0.13-analytics-improvements branch from a082b5c to ecf8caf Compare May 11, 2023 08:24
@mkhq mkhq force-pushed the 0.13-analytics-improvements branch from 4c15109 to c9d0388 Compare May 11, 2023 15:38
Copy link
Collaborator

@eysi09 eysi09 left a comment

Choose a reason for hiding this comment

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

Two comments, otherwise looks great! 🙏

@@ -92,6 +94,11 @@ interface ProjectMetadata {
servicesCount: number
testsCount: number
moduleTypes: string[]
actionsCount: number
actionBuildsCount: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick but feels like this should be called buildActionCount, deployActionCount, etc

@@ -125,6 +132,7 @@ interface CommandEvent extends EventBase {
type: "Run Command"
properties: PropertiesBase & {
name: string
commandIteration: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should skip this for now since we're changing this behaviour in general:

  • Introduce the concept of a parent session (with a parentSessionId)
  • This would be a persistent session like garden dev
  • Every time you run a command within that session, via the terminal / Cloud / Desktop, that gets counted as a single session within the parent session.
  • With this, a command can't technically be run twice. Each session is exactly a single command but it can have a parent command. Only persistent commands like garden dev can be parent commands.

@mkhq mkhq requested a review from eysi09 May 12, 2023 06:27
eysi09
eysi09 previously approved these changes May 12, 2023
@mkhq
Copy link
Contributor Author

mkhq commented May 12, 2023

@eysi09 I've re-enabled the failing nock tests, if this turns out to be an issue still we can re-add the skip.

@mkhq mkhq requested a review from eysi09 May 12, 2023 08:49
@mkhq
Copy link
Contributor Author

mkhq commented May 12, 2023

@eysi09 I've re-enabled the failing nock tests, if this turns out to be an issue still we can re-add the skip.

Added the skip back because those tests keep on failing.

@mkhq mkhq merged commit c55c31d into 0.13 May 12, 2023
@mkhq mkhq deleted the 0.13-analytics-improvements branch May 12, 2023 10:14
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.

2 participants