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

Revert "[image_picker] add forceFullMetadata to interface" #4314

Merged
merged 3 commits into from
Sep 6, 2021

Conversation

zanderso
Copy link
Member

@zanderso zanderso commented Sep 5, 2021

Reverts #4288

FYI @cpboyd

This commit appears to be breaking the framework tree somehow: https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20flutter_plugins/3301/overview

Running command: "dart analyze --fatal-infos" in /b/s/w/ir/x/t/flutter_plugins.DYUYDK/packages/image_picker/image_picker_for_web
Analyzing image_picker_for_web...

  error - lib/image_picker_for_web.dart:51:22 - 'ImagePickerPlugin.pickImage' ('Future<PickedFile> Function({int? imageQuality, double? maxHeight, double? maxWidth, CameraDevice preferredCameraDevice, required ImageSource source})') isn't a valid override of 'ImagePickerPlatform.pickImage' ('Future<PickedFile?> Function({bool forceFullMetadata, int? imageQuality, double? maxHeight, double? maxWidth, CameraDevice preferredCameraDevice, required ImageSource source})'). - invalid_override
  error - lib/image_picker_for_web.dart:113:17 - 'ImagePickerPlugin.getImage' ('Future<XFile> Function({int? imageQuality, double? maxHeight, double? maxWidth, CameraDevice preferredCameraDevice, required ImageSource source})') isn't a valid override of 'ImagePickerPlatform.getImage' ('Future<XFile?> Function({bool forceFullMetadata, int? imageQuality, double? maxHeight, double? maxWidth, CameraDevice preferredCameraDevice, required ImageSource source})'). - invalid_override

2 issues found.

Fixes flutter/flutter#89517

@@ -4,7 +4,7 @@ repository: https://github.com/flutter/plugins/tree/master/packages/image_picker
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+image_picker%22
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 2.4.0
version: 2.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

You can't revert the version, because 2.3.0 and 2.4.0 are already published. This should be 2.4.1.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@stuartmorgan
Copy link
Contributor

This commit appears to be breaking the framework tree somehow

It published a breaking change as if it weren't breaking.

@stuartmorgan
Copy link
Contributor

The reasons things are red is because it's using the published, broken version of the interface package to analyze web. I think we'll need to force-land this and break the tree temporarily, then publish on red, to fix it. This is quite a mess.

I will take a look later.

@stuartmorgan
Copy link
Contributor

I think I may be able to fix this forward in a way that won't require bypassing all of our testing safeties; let me see if that will work.

@stuartmorgan
Copy link
Contributor

I think I may be able to fix this forward in a way that won't require bypassing all of our testing safeties; let me see if that will work.

No, I forgot that Dart doesn't allow C-style function overloading. That makes what I had in mind impossible.

I think I just have to push this through a bunch of red. I'll manually check all the failures here and make sure they are exactly what I expect from this situation.

@stuartmorgan
Copy link
Contributor

publishable was a legitimate failure, which I've fixed. The rest is in the clients of the interface package as expected for this scenario. I'll let publishable re-run to completion, then force this through if that's green.

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. I will force land this, which will in turn turn the tree red. I'll then manually publish image_picker_platform_interface, which should fix everything (including flutter/flutter) and then re-run the post-submit tests to green the tree.

(Actually, if I publish fast enough I might be able to beat at least some of the post-submit tests...)

@stuartmorgan stuartmorgan merged commit 3a38cd0 into master Sep 6, 2021
@stuartmorgan stuartmorgan deleted the revert-4288-int-metadata branch September 6, 2021 00:24
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 6, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 7, 2021
NickalasB added a commit to NickalasB/plugins that referenced this pull request Sep 7, 2021
* master: (490 commits)
  Allow neutral conclusion in publishing check (flutter#4321)
  Revert "[image_picker] add forceFullMetadata to interface" (flutter#4314)
  [image_picker] add forceFullMetadata to interface (flutter#4288)
  [flutter_plugin_tools] Adjust diff logging (flutter#4312)
  Remove gradle.properties from plugins (flutter#4309)
  build-examples .pluginToolsConfig.yaml support (flutter#4305)
  [camera_web] Make plugin publishable for the first time. (flutter#4304)
  adds option to re-authenticate on google silent sign in (flutter#4251)
  [ci.yaml] Add builders to recipes cq (flutter#4306)
  Add scripts for Windows LUCI recipe (flutter#4303)
  [video_player] Ensure seekTo is not called before video player is initialized.  (flutter#4300)
  Ensure setExposureOffset returns new value on Android (flutter#4301)
  Add a way to opt a file out of Dart formatting (flutter#4292)
  [flutter_plugin_tools] Add Linux support to native-test (flutter#4294)
  [image_picker]Android update cache (flutter#4124)
  [flutter_plugin_tools] Fix build-examples for packages (flutter#4248)
  [flutter_plugin_tool] Migrate 'publish' to new base command (flutter#4290)
  [flutter_plugin_tool] Move branch-switching logic from tool_runner.sh to tool (flutter#4268)
  [flutter_plugin_tool] Add support for running Windows unit tests (flutter#4276)
  [camera_web] Do not flip the video on the back camera (flutter#4281)
  ...

# Conflicts:
#	packages/webview_flutter/webview_flutter/CHANGELOG.md
#	packages/webview_flutter/webview_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebView.java
#	packages/webview_flutter/webview_flutter/pubspec.yaml
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 7, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
)

Reverts flutter#4288 which published a breaking change as if it were a non-breaking change.

The discussion in that PR prior to it landing was incorrect, because adding a new parameter
with a default value is non-breaking *only for clients*. It is breaking for subclasses that
override it, and the purpose of the platform interface is for implementations to subclass it
and override everything.
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
)

Reverts flutter#4288 which published a breaking change as if it were a non-breaking change.

The discussion in that PR prior to it landing was incorrect, because adding a new parameter
with a default value is non-breaking *only for clients*. It is breaking for subclasses that
override it, and the purpose of the platform interface is for implementations to subclass it
and override everything.
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
)

Reverts flutter#4288 which published a breaking change as if it were a non-breaking change.

The discussion in that PR prior to it landing was incorrect, because adding a new parameter
with a default value is non-breaking *only for clients*. It is breaking for subclasses that
override it, and the purpose of the platform interface is for implementations to subclass it
and override everything.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux flutter_plugins broken on unrelated commit
2 participants