Skip to content

Conversation

@eliasyishak
Copy link
Contributor

Fixes:


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@eliasyishak eliasyishak linked an issue Sep 18, 2023 that may be closed by this pull request
@eliasyishak eliasyishak changed the title 162 expose the clientid Expose the client id + hard code for NoOp instance Sep 18, 2023

/// The shared identifier for Flutter and Dart related tooling using
/// package:unified_analytics.
String get clientId;
Copy link
Contributor

Choose a reason for hiding this comment

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

is this addition going to break existing clients? if so, we better bump the major version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good call, seems like the analysis server is implementing our NoOp instance so I will bump the version to 4.0.0

https://github.com/dart-lang/sdk/blob/main/pkg/analysis_server/test/src/analytics/analytics_manager_test.dart#L559-L564

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

Code lgtm, just a question about if we should bump major version

@eliasyishak eliasyishak merged commit 70d778d into dart-lang:main Sep 19, 2023
@eliasyishak eliasyishak deleted the 162-expose-the-clientid branch September 19, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose the clientId on the Analytics instance

2 participants