-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having a hard time understanding the rationale for bumping the minimum version here.
Can you provide a bit more detail on why this is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Some comments inline below.
@stuartmorgan @cbracken Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (I made a small style fix since the verb form didn't match our style).
@cbracken for secondary review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.
This PR updates the
win32
dependency to3.0.0
to avoid current and future version conflicts.List which issues are fixed by this PR. You must list at least one issue.
ThexXTURBOXx/flutter_web_auth_2#6
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.