Skip to content

Migration to null safety#11

Merged
kevin49999 merged 5 commits intopolicygenius:mainfrom
afermin:migration_null_safety
Apr 20, 2021
Merged

Migration to null safety#11
kevin49999 merged 5 commits intopolicygenius:mainfrom
afermin:migration_null_safety

Conversation

@afermin
Copy link
Copy Markdown
Contributor

@afermin afermin commented Apr 20, 2021 •
edited
Loading

Reopened:

Hi there,

I just migrated to null safety the plugin regarding this #10

I hope you can take a look and left some comments if needed. @maxnovak @longhorn499

Best.

Description

Motivation and Context

It's required to be able to migrate to dart null safety.

10

Type of Change:

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Maintenance or Refactor

Checklist

  • I added tests to cover my changes
  • I rebased changes with master so that they can be merged easily
  • I have updated the documentation accordingly (if applicable)

@afermin afermin mentioned this pull request Apr 20, 2021
Closed
@kevin49999 kevin49999 self-requested a review April 20, 2021 19:20
@afermin
Merge branch 'main' into migration_null_safety
0900d69 
# Conflicts:
#	lib/optimizely_plugin.dart
#	pubspec.yaml
maxnovak
maxnovak reviewed Apr 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@maxnovak maxnovak left a comment

Choose a reason for hiding this comment

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

Could you please undo the format changes, which are unrelated to your PR?
e.g. the double to single quotes & tab changes

@afermin
Copy link
Copy Markdown
Contributor Author

afermin commented Apr 20, 2021

Could you please undo the format changes, which are unrelated to your PR?
e.g. the double to single quotes & tab changes

Done

@afermin
Copy link
Copy Markdown
Contributor Author

afermin commented Apr 20, 2021

Looks like the flutter version should updated in the CI

kevin49999
kevin49999 reviewed Apr 20, 2021
View reviewed changes
kevin49999
kevin49999 reviewed Apr 20, 2021
View reviewed changes
kevin49999
kevin49999 reviewed Apr 20, 2021
View reviewed changes
@kevin49999
Copy link
Copy Markdown
Contributor

kevin49999 commented Apr 20, 2021 •
edited
Loading

Looks like the flutter version should updated in the CI

Yep, I think updating it to 2.0.4 (or any newer version past the latest major release) should do it there. You can update that in .github/workflows/ci.yaml and if that passes locally I can merge this in and do the release. Not sure if updating that on this branch will cause the action here to run with the higher version (or it will just run w/ the action settings on main) 🤔 but I can pull it locally if not and merge / follow-up and fix the action

@afermin
Copy link
Copy Markdown
Contributor Author

afermin commented Apr 20, 2021

Looks like the flutter version should updated in the CI

Yep, I think updating it to 2.0.4 (or any newer version past the latest major release) should do it there. You can update that in .github/workflows/ci.yaml and if that passes locally I can merge this in and do the release. Not sure if updating that on this branch will cause the action here to run with the higher version (or it will just run w/ the action settings on main) 🤔 but I can pull it locally if not and merge / follow-up and fix the action

Thanks for that. Already changed it and it worked

@kevin49999 kevin49999 self-requested a review April 20, 2021 21:07
kevin49999
kevin49999 approved these changes Apr 20, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@kevin49999 kevin49999 left a comment

Choose a reason for hiding this comment

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

lgtm! 🌠 Thanks for contributing

@kevin49999 kevin49999 merged commit 46b2ed4 into policygenius:main Apr 20, 2021
@afermin
Copy link
Copy Markdown
Contributor Author

afermin commented Apr 20, 2021

lgtm! 🌠 Thanks for contributing

My pleasure. Thank you for letting me do that!

@kevin49999
Copy link
Copy Markdown
Contributor

kevin49999 commented Apr 20, 2021 •
edited
Loading

lgtm! 🌠 Thanks for contributing

My pleasure. Thank you for letting me do that!

Of course, thanks again. It's published. There are notes in the release on GitHub, but I will follow-up with an update in the CHANGELOG as well as fix some warnings I found when running dart pub publish --dry-run

@kevin49999 kevin49999 mentioned this pull request Apr 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@maxnovak maxnovak maxnovak left review comments

@kevin49999 kevin49999 kevin49999 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@afermin @kevin49999 @maxnovak