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

[camera] Switch web package to new analysis options #4834

Merged
merged 9 commits into from
Feb 16, 2022

Conversation

stuartmorgan
Copy link
Contributor

Switches from legacy analysis options to current analysis options,
fixing all analysis issues it exposed.

Part of flutter/flutter#76229

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.

Switches from legacy analysis options to current analysis options,
fixing all analysis issues it exposed.

Part of flutter/flutter#76229
@stuartmorgan stuartmorgan marked this pull request as ready for review February 14, 2022 19:16
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

So many missing types, omg. I think I only found one maybe-typo with a list of lists below (which probably doesn't matter too much) 🚀

Comment on lines +237 to +238
(videoTrackCapabilities[_facingModeKey] as List<dynamic>?)
?.cast<String>() ??
Copy link
Member

Choose a reason for hiding this comment

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

why the explicit .cast() here? can't this be done at the same time as the as above?

videoTrackCapabilities[_facingModeKey] as List<String>?) ?? <String>[]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. This is an example of what I was talking about in the other comment. dynamic is castable to String, but List<dynamic> isn't castable to List<String>.

See https://dartpad.dev/?id=bb6c33398890a6b30c5c5a3db5bdf27f for an example.

Copy link
Member

Choose a reason for hiding this comment

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

Ahhh I understand, yikes!

Copy link
Member

@ditman ditman Feb 16, 2022

Choose a reason for hiding this comment

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

This however works in the same Gist:

  List<Object?> bad = allStrings as List<Object?>; // "as" not even needed!

I think dynamic can be handled as Object? to mitigate what lrhn is commenting here.

(I'm sorry I can't find any better resource than that comment, I'd swear I had read about "Using Object? instead of dynamic" somewhere :S)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, I'll need to research more. I've definitely been applying the "always use Object? instead of dynamic" guidance in all other cases, but I've been so wary of things like JSON and method channels after screwing it up several times that I just retreated to "assume that since it's coming in as dynamic only dynamic is safe", which it sounds like swung too far.

@stuartmorgan stuartmorgan added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Feb 16, 2022
@fluttergithubbot fluttergithubbot merged commit 72ebbf3 into flutter:main Feb 16, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2022
debokarmakar pushed a commit to nurture-farm/plugins that referenced this pull request Feb 17, 2022
* google/master: (340 commits)
  [camera]remove "selfRef" for SavePhotoDelegate and ensure thread safety (flutter#4780)
  Roll Flutter from 919d205 to adafd66 (5 revisions) (flutter#4876)
  [google_sign_in] Update platform interface analysis options (flutter#4872)
  Roll Flutter from b623279 to 919d205 (2 revisions) (flutter#4871)
  Roll Flutter from 14a2b13 to b623279 (5 revisions) (flutter#4870)
  [image_picker] Update platform interface analysis options (flutter#4837)
  [ci] Re-enable stable webview Android tests (flutter#4867)
  Roll Flutter from 286c975 to 14a2b13 (1 revision) (flutter#4865)
  [local_auth] support localizedFallbackTitle in IOSAuthMessages (flutter#3806)
  [image_picker] Update app-facing and web analysis options (flutter#4838)
  Roll Flutter from 8386344 to 286c975 (4 revisions) (flutter#4864)
  Roll Flutter from e9f83cf to 8386344 (6 revisions) (flutter#4862)
  [camera] Switch web package to new analysis options (flutter#4834)
  [flutter_plugin_tool] Fix iOS/macOS naming (flutter#4861)
  [webview_flutter] Fix debuggingEnabled on Android (flutter#4859)
  Roll Flutter from ca2a751 to e9f83cf (10 revisions) (flutter#4857)
  fix license (flutter#4858)
  [video_player] add allowBackgroundPlayback option platform interface changes (flutter#4807)
  [video_player] Updated Pigeon version (flutter#4726)
  Roll Flutter from 381cb28 to ca2a751 (5 revisions) (flutter#4850)
  ...

# Conflicts:
#	packages/camera/camera/CHANGELOG.md
#	packages/camera/camera/android/build.gradle
#	packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/Camera.java
#	packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/MethodCallHandlerImpl.java
#	packages/camera/camera/ios/Classes/CameraPlugin.m
#	packages/camera/camera/ios/camera.podspec
#	packages/camera/camera/lib/camera.dart
#	packages/camera/camera/lib/src/camera_controller.dart
#	packages/camera/camera/pubspec.yaml
@stuartmorgan stuartmorgan deleted the analysis-camera-web branch April 19, 2022 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p: camera platform-web waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants