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

[camera] Remove deprecated Optional type #6870

Merged
merged 6 commits into from
Dec 20, 2022
Merged

[camera] Remove deprecated Optional type #6870

merged 6 commits into from
Dec 20, 2022

Conversation

camsim99
Copy link
Contributor

@camsim99 camsim99 commented Dec 20, 2022 •
edited
Loading

Fixes flutter/flutter#117403.

Removes deprecated Optional type from camera plugin to fix failing framework Linux_plugins test., e.g. https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8794239165172938017/+/u/run_test.dart_for_flutter_plugins_shard_and_subshard_analyze/test_stdout.

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.

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

@jmagman
Copy link
Member

jmagman commented Dec 20, 2022 

  error - example/lib/camera_controller.dart:128:33 - The argument type 'dynamic' can't be assigned to the parameter type 'DeviceOrientation?'. - argument_type_not_assignable
  error - example/lib/camera_controller.dart:130:38 - The getter 'orNull' isn't defined for the type 'DeviceOrientation'. Try importing the library that defines 'orNull', correcting the name to the name of an existing getter, or defining a getter or field named 'orNull'. - undefined_getter
  error - example/lib/camera_controller.dart:131:29 - The argument type 'dynamic' can't be assigned to the parameter type 'DeviceOrientation?'. - argument_type_not_assignable
  error - example/lib/camera_controller.dart:133:34 - The getter 'orNull' isn't defined for the type 'DeviceOrientation'. Try importing the library that defines 'orNull', correcting the name to the name of an existing getter, or defining a getter or field named 'orNull'. - undefined_getter
  error - example/lib/camera_controller.dart:135:32 - The argument type 'dynamic' can't be assigned to the parameter type 'DeviceOrientation?'. - argument_type_not_assignable
  error - example/lib/camera_controller.dart:137:37 - The getter 'orNull' isn't defined for the type 'DeviceOrientation'. Try importing the library that defines 'orNull', correcting the name to the name of an existing getter, or defining a getter or field named 'orNull'. - undefined_getter
   info - example/lib/camera_controller.dart:11:8 - Unused import: 'package:quiver/core.dart'. Try removing the import directive. - unused_import
   info - example/lib/camera_controller.dart:273:73 - Avoid redundant argument values. - avoid_redundant_argument_values
   info - example/lib/camera_controller.dart:332:29 - Avoid redundant argument values. - avoid_redundant_argument_values
   info - example/lib/camera_controller.dart:407:54 - Avoid redundant argument values. - avoid_redundant_argument_values

Are you able to see the analysis errors locally?

@christopherfujino
Copy link
Member

@camsim99 FYI https://github.com/google/quiver-dart/pull/717/files, the flutter/flutter tree is unbroken. Probably still worth removing Optional (it will eventually be deprecated), but at least this isn't a tree blocker

@zanderso
Copy link
Member

quiver will 3.2.1 un-deprecate Optional google/quiver-dart#717, but it will be deprecated again soon.

@camsim99 camsim99 marked this pull request as ready for review December 20, 2022 19:25
christopherfujino
christopherfujino reviewed Dec 20, 2022
View reviewed changes
packages/camera/camera/pubspec.yaml Outdated
@@ -4,7 +4,7 @@ description: A Flutter plugin for controlling the camera. Supports previewing
Dart.
repository: https://github.com/flutter/plugins/tree/main/packages/camera/camera
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+camera%22
version: 0.10.0+5
version: 0.10.0+6
Copy link
Member

Choose a reason for hiding this comment

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

should we bump this to 0.11 since we're changing a public API?

christopherfujino
christopherfujino approved these changes Dec 20, 2022
View reviewed changes
Copy link
Member

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

cod changes LGTM. This seems like we're changing a public API though, so I think we should bump the y part of the versions.

jmagman
jmagman approved these changes Dec 20, 2022
View reviewed changes
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 20, 2022
@auto-submit auto-submit bot merged commit 3d8b73b into flutter:main Dec 20, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 21, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 21, 2022
@engine-flutter-autoroll
3d8b73bf0 [camera] Remove deprecated Optional type (flutter/plugins#6870
aeda65f 
)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 21, 2022
@engine-flutter-autoroll
Roll Plugins from 840a04954fa0 to 54fc2066d636 (6 revisions) (#117456)
a7a5d14 
* 697c8b3ee [image_picker_ios] Pass through error message from image saving (flutter/plugins#6858)

* 72f810851 [local_auth] Bump `intl` from ^0.17.0 to ">=0.17.0 <0.19.0" (flutter/plugins#6848)

* acbe9b452 [gh_actions]: Bump github/codeql-action from 2.1.35 to 2.1.37 (flutter/plugins#6860)

* 3d8b73bf0 [camera] Remove deprecated Optional type (flutter/plugins#6870)

* c5220efae [in_app_purchase] Add support for macOS (flutter/plugins#6519)

* 54fc2066d Roll Flutter from 028c6e2 to dbc9306 (11 revisions) (flutter/plugins#6849)
AsturaPhoenix
AsturaPhoenix reviewed Dec 23, 2022
View reviewed changes
packages/camera/camera/lib/src/camera_controller.dart
@@ -372,9 +367,7 @@ class CameraController extends ValueNotifier<CameraValue> {
}
try {
await CameraPlatform.instance.resumePreview(_cameraId);
value = value.copyWith(
isPreviewPaused: false,
previewPauseOrientation: const Optional<DeviceOrientation>.absent());
Copy link

Choose a reason for hiding this comment

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

Unfortunately this seems to break camera orientation changes while preview is paused. Since copyWith can no longer be told to clear previewPauseOrientation, it keeps its stale value and locks the CameraPreview widget in the old orientation until the next pausePreview call.

A few of the other fields appear to be nullable as well, but I haven't looked closely to see if there are any other regressions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh ok, sorry I didn't realize this. I may have to do some refactoring where Optional<DeviceOrientation> was used. Thanks for letting me know!

AsturaPhoenix added a commit to AsturaPhoenix/flutter-plugins that referenced this pull request Dec 23, 2022
@AsturaPhoenix
Revert "[camera] Remove deprecated Optional type (flutter#6870)"
050f8a9 
Removing Optional from CameraValue.copyWith caused a regression handling device orientation changes while the preview was paused.

This reverts commit 3d8b73b.
@camsim99 camsim99 mentioned this pull request Jan 3, 2023
Merged
11 tasks
camsim99 added a commit to camsim99/plugins that referenced this pull request Jan 17, 2023
@camsim99
Revert "[camera] Remove deprecated Optional type (flutter#6870)"
03f992c 
This reverts commit 3d8b73b.
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
@engine-flutter-autoroll @gspencergoog
Roll Plugins from 840a04954fa0 to 54fc2066d636 (6 revisions) (flutter…
c3af831 
…#117456)

* 697c8b3ee [image_picker_ios] Pass through error message from image saving (flutter/plugins#6858)

* 72f810851 [local_auth] Bump `intl` from ^0.17.0 to ">=0.17.0 <0.19.0" (flutter/plugins#6848)

* acbe9b452 [gh_actions]: Bump github/codeql-action from 2.1.35 to 2.1.37 (flutter/plugins#6860)

* 3d8b73bf0 [camera] Remove deprecated Optional type (flutter/plugins#6870)

* c5220efae [in_app_purchase] Add support for macOS (flutter/plugins#6519)

* 54fc2066d Roll Flutter from 028c6e2 to dbc9306 (11 revisions) (flutter/plugins#6849)
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
@camsim99 @mauricioluz
[camera] Remove deprecated Optional type (flutter#6870)
61955d3 
* Remove Optional

* Undo accidental order change

* Fix examples analyze

* Remove unused import

* Bump versions

* Correct version
auto-submit bot pushed a commit that referenced this pull request Jan 26, 2023
@camsim99
[camera] Add back Optional type for nullable CameraController orienta…
ff84c44 
…tions (#6911)

* Add flag

* Add missing comment

* Add tests

* Bump versions

* Stage changelog changes

* Revert "Fix examples analyze"

This reverts commit 4db1858.

* Revert "[camera] Remove deprecated Optional type (#6870)"

This reverts commit 3d8b73b.

* Add back optional

* Edit changelog

* Fix semicolon

* Add )
miracoli added a commit to miracoli/plugins that referenced this pull request Feb 10, 2023
@miracoli
Revert "[camera] Remove deprecated Optional type (flutter#6870)"
d04a7ec 
Removing Optional from CameraValue.copyWith caused a regression handling device orientation changes while the preview was paused.

This reverts commit 3d8b73b.

# Conflicts:
#	packages/camera/camera/CHANGELOG.md
#	packages/camera/camera/lib/src/camera_controller.dart
#	packages/camera/camera/pubspec.yaml
#	packages/camera/camera_android/CHANGELOG.md
#	packages/camera/camera_android/pubspec.yaml
#	packages/camera/camera_avfoundation/CHANGELOG.md
#	packages/camera/camera_avfoundation/pubspec.yaml
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AsturaPhoenix AsturaPhoenix AsturaPhoenix left review comments

@jmagman jmagman jmagman approved these changes

@christopherfujino christopherfujino christopherfujino approved these changes

@hellohuanlin hellohuanlin Awaiting requested review from hellohuanlin hellohuanlin is a code owner

@bparrishMines bparrishMines Awaiting requested review from bparrishMines bparrishMines is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Linux flutter_plugins failing on new deprecation from package:quiver
5 participants
@camsim99 @jmagman @christopherfujino @zanderso @AsturaPhoenix