The Wayback Machine - https://web.archive.org/web/20221216163930/https://github.com/flutter/plugins/pull/6847
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

Jump to bottom

[camera][google_maps_flutter][path_provider] prepare for TestDefaultBinaryMessengerBinding.instance becoming non-nullable #6847

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[camera][google_maps_flutter][path_provider] prepare for TestDefaultBinaryMessengerBinding.instance becoming non-nullable #6847

wants to merge 1 commit into from

Conversation

Hixie
Copy link
Member

@Hixie Hixie commented Dec 15, 2022

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.
@Hixie Hixie added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Dec 15, 2022
@Hixie
Copy link
Member Author

Hixie commented Dec 15, 2022

For flutter/flutter#116937
stuartmorgan
stuartmorgan reviewed Dec 15, 2022
View changes
packages/camera/camera_android/test/android_camera_test.dart
// This should be removed once the relevant changes makes it to stable.
// TODO(ianh): look into this in Q2 2023.
T _ambiguate<T extends Object>(T? value) {
return value!;
Copy link
Contributor

@stuartmorgan stuartmorgan Dec 15, 2022

Choose a reason for hiding this comment

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

Is it intentional that this is the reverse of what we did last time? We did T->T? and then kept the ! at the call site before, which seems more explicit about what's happening at the call site. If we want this behavior instead I think we should call this something that sounds dangerous, since it's force-unwrapping (e.g., _forceUnwrapIfNullable).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: camera p: google_maps_flutter p: path_provider platform-android platform-ios platform-macos
2 participants
@Hixie @stuartmorgan