Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[camera] Stable release for null safety. #3641

Merged
merged 8 commits into from
Mar 2, 2021

Conversation

mvanbeusekom
Copy link
Contributor

Stable NNBD release for the camera package.

With this PR I also migrated the example App to sound null safe. Note that I didn't update the dev-dependency on mockito: ^5.0.0-nullsafety.7 since the stable version causes a conflict with the current flutter_driver dependency. I have marked the dependency in the pubspec.yaml with a # TODO(mvanbeusekom) comment to remind me to remove it when the conflict is resolved.

Conflict details:

Because mockito >=5.0.0 depends on analyzer ^1.0.0 which depends on crypto ^3.0.0, mockito >=5.0.0 requires crypto ^3.0.0.
And because every version of flutter_driver from sdk depends on crypto 2.1.5, mockito >=5.0.0 is incompatible with flutter_driver from sdk.
So, because camera depends on both flutter_driver any from sdk and mockito ^5.0.0, version solving failed.

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

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • 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 Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

packages/camera/camera/example/lib/main.dart Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Show resolved Hide resolved
# Because mockito >=5.0.0 depends on analyzer ^1.0.0 which depends on crypto ^3.0.0
# every version of flutter_driver from sdk depends on crypto 2.1.5.
mockito: ^5.0.0-nullsafety.7
plugin_platform_interface: ^2.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

I think since this is the last plugin to be published as stable, this will work. Let's see how our bots handle it.

(Ideally we can now sweep the repo to make everything require 2.0.0 instead of a range that includes a non-nullable version.)

@mvanbeusekom
Copy link
Contributor Author

@stuartmorgan thanks for the feedback. I have gone through all your comments and made the necessary adjustments.

Could you have another look please?

@stuartmorgan stuartmorgan added the nnbd Support for null safety label Mar 1, 2021
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

A few more nits, but otherwise looks good.

packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
packages/camera/camera/example/lib/main.dart Outdated Show resolved Hide resolved
pedantic: ^1.8.0
integration_test:
path: ../../../integration_test
test: any
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this to resolve a conflict? We don't usually use any unless we have to; I would expect 1.16.0 (or 1.16.3 if you need Fake).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was actually a mistake on my end. I added this dependency because I was receiving a CI failure with the dependencies related to the integration_test in combination with running in NNBD. I started comparing with the video_player plugin and noticed that this dependency was included this way and decided to give that a try. Reading back in the Discord channel I realised that we shouldn't run integration_tests in NNBD mode (and add the // @dart = 2.9 instruction to the top of the file). Unfortunately I forgot to remove this dependency which I did this time ;).

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM!

@stuartmorgan stuartmorgan merged commit a6cbf67 into flutter:master Mar 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 2, 2021
fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request Mar 2, 2021
NickalasB added a commit to NickalasB/plugins that referenced this pull request Mar 3, 2021
* master:
  Adopt Xcode 12 for podspec lints (flutter#3653)
  Run static analyzer during xctest (flutter#3667)
  [google_maps_flutter_web] update min flutter sdk version to 1.20.0 (flutter#3662)
  [image_picker] Run CocoaPods iOS tests in RunnerUITests target (flutter#3663)
  [webview_flutter] Run CocoaPods iOS tests in RunnerUITests target (flutter#3664)
  [device_info] Enable NNBD for unit test (flutter#3658)
  remove unused plugin (flutter#3661)
  [android_intent] move unit test to nullsafety (flutter#3659)
  [url_launcher] Migrate unit tests to NNBD (flutter#3657)
  [share] Migrate unit tests to null-safety. (flutter#3660)
  [connectivity_for_web] Migration to null-safety. (flutter#3652)
  [camera] Stable release for null safety. (flutter#3641)
  [in_app_purchase] fix plugin version (flutter#3654)
  Move plugin tool tests over (flutter#3606)
  [in_app_purchase] migrate playing billing library to v3 (flutter#3636)
  Update plugin_platform_interface min version (flutter#3650)

# Conflicts:
#	packages/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/example/ios/Runner.xcodeproj/project.pbxproj
@mvanbeusekom mvanbeusekom deleted the stable_nnbd_camera branch September 21, 2021 09:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes nnbd Support for null safety p: camera
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants