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

Migrate pub to use package:unified_analytics #4031

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Conversation

bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Oct 20, 2023

This is some prerequisite work for migrating package:dartdev to use package:unified_analytics.

I've removed the single analytics test here in favour of the dart pub get analytics test in dartdev for simplicity, as package:unified_analytics adds a FakeAnalytics class that tracks analytics events for easy validation in tests without needing to manually serialize all the analytics events. It's not possible to serialize / deserialize FakeAnalytics so the results can't be verified from a separate process at this point, so this testing is easier to do in dartdev. If you'd prefer we keep this testing here, please let me know if you have ideas on how to move forward 😊

@bkonyi bkonyi requested review from jonasfj and sigurdm October 20, 2023 14:19
Copy link
Contributor

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

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

LGTM but would love to have some test coverage

import 'dart:io';

import 'package:path/path.dart' as path;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

test/embedding/embedding_test.dart Show resolved Hide resolved
@bkonyi bkonyi merged commit 90ef5f9 into master Oct 31, 2023
23 checks passed
@bkonyi bkonyi deleted the unified_analytics branch October 31, 2023 14:45
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