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
Analytics: track integration environment #1479
Conversation
829cf19
to
6f0b392
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good other than needing to modify test/analytics.test.ts
to add
integrationEnvironment
and integrationEnvironmentVersion
to a few spots to fix the tests.
test/analytics-sources.spec.ts
Outdated
@@ -162,3 +168,41 @@ describe('analytics-sources - getIntegrationVersion', () => { | |||
).toBe('1.2.3-Crystal'); | |||
}); | |||
}); | |||
|
|||
describe('analytics-sources - getIntegrationEnvironment', () => { | |||
it('integration environment is empty by default', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick and I'm by no means perfect at this myself (naming is hard), but I understand that one should try to make the describe + the it sort of flow like a sentence. So you could make this
it('returns empty environment by default', () => {
Then you'd have getIntegrationEnvironment returns empty environment by default
.
Similarly for the others.
6f0b392
to
a63850d
Compare
This will help us capture if CLI is running e.g. in a specific IDE version
a63850d
to
8d4f267
Compare
Rebased, simplified the function to always empty empty string instead of undefined, just to be consistent with the |
🎉 This PR is included in version 1.419.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This will help us capture debug/analytics data if CLI is running e.g. in a specific IDE version.