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 to flutter 2 #10

Merged
merged 3 commits into from
Jun 4, 2021
Merged

Migrate to flutter 2 #10

merged 3 commits into from
Jun 4, 2021

Conversation

jonataslaw
Copy link
Contributor

Hi, I noticed that the project became incompatible with one of our projects after the Flutter 2 update, so I migrated it

Copy link
Contributor

@yakkomajuri yakkomajuri left a comment

Choose a reason for hiding this comment

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

Hey, thank you so much for doing this! We don't have a dedicated Dart/Flutter dev hence it took a while to get around to this.

Left a few minor comments but I'm still testing this out

Map<String, dynamic> properties,
Map<String, dynamic> options,
required String eventName,
required Map<String, dynamic> properties,
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems to have diverged from lib/src/posthog_method_channel.dart where properties are correctly assigned as optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I'm going to make it nullable instead of required

await Posthog.enable();
Posthog.capture(eventName: 'Enabled capturing events!');
await Posthog().enable();
Posthog().capture(
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 standard Dart indentation? (I don't know so I'm truly asking)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's pretty odd for those who are used to other languages, but it's the default, I even used dart format in this file before saving the changes

@yakkomajuri
Copy link
Contributor

@jonataslaw this worked for me and all the changes make sense! Thanks again for doing this. A few minor comments otherwise happy to merge.

@jonataslaw
Copy link
Contributor Author

I just changed properties to nullable to be consistent with posthog_method_channel now

Copy link
Contributor

@yakkomajuri yakkomajuri left a comment

Choose a reason for hiding this comment

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

Thanks again! Will get this in but I don't have access to publish a new version, will look to get access to this internally

@fuziontech
Copy link
Member

lib pushed to pub.dev!
https://pub.dev/packages/posthog_flutter

Available version is now 2.0.0

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.

4 participants