Skip to content

Update other session file create callsite #256

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

Conversation

eliasyishak
Copy link
Contributor

  • Thanks for your contribution! Please replace this text with a description of what this PR is changing or adding and why, list any relevant issues, and review the contribution guidelines below.

  • 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.

Copy link

Package publishing

Package Version Status Publish tag (post-merge)
package:unified_analytics 5.8.8 ready to publish unified_analytics-v5.8.8
package:cli_config 0.2.0 already published at pub.dev
package:extension_discovery 2.0.1-wip WIP (no publish necessary)
package:graphs 2.3.2-wip WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@@ -640,7 +640,7 @@ class AnalyticsImpl implements Analytics {
// Recreate the session and client id file; no need to
// recreate the log file since it will only receives events
// to persist from events sent
Initializer.createClientIdFile(clientFile: _clientIdFile);
Initializer.createClientIdFile(clientIdFile: _clientIdFile);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a small correction to the spelling

@@ -49,7 +49,8 @@ class Session {
if (now.difference(lastPingDateTime).inMinutes > kSessionDurationMinutes) {
// Update the session file with the latest session id
_sessionId = now.millisecondsSinceEpoch;
sessionFile.writeAsStringSync('{"session_id": $_sessionId}');
sessionFile.writeAsStringSync(
Copy link
Contributor

Choose a reason for hiding this comment

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

can we create a method that does this write, so we make sure to always keep the multiple call sites in sync?

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.

nit to abstract the multliple writes to a method, but if you want to merge, publish, and then follow up with the refactor that SGTM.

@eliasyishak eliasyishak merged commit f611290 into dart-lang:main Mar 15, 2024
@eliasyishak eliasyishak deleted the update-other-session-file-create-callsite branch March 15, 2024 13:32
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.

2 participants