This repository was archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[camera] Simplest camera plugin migration #2118
Closed
matthew-carroll
wants to merge
4
commits into
flutter:master
from
matthew-carroll:simplest_camera_plugin_migration
Closed
[camera] Simplest camera plugin migration #2118
matthew-carroll
wants to merge
4
commits into
flutter:master
from
matthew-carroll:simplest_camera_plugin_migration
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ded new example Activity that uses new embedding - still needs lots of refactoring and tests.
…herwise a really terrible quality of code that's untestable.
amirh
reviewed
Sep 30, 2019
|
|
||
| dependencies { | ||
| // TODO: remove these dependencies when engine POM is working correctly. | ||
| implementation 'androidx.lifecycle:lifecycle-runtime:2.1.0' |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if there's another plugin that requires an incompatible version of lifecycle (e.g 1.1.1) before having the POM?
What's the timeline for the POM?
cc @blasten
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need these dependencies. @matthew-carroll
13 tasks
mklim
pushed a commit
that referenced
this pull request
Oct 15, 2019
Migrates the camera plugin to the new embedding and adds an example activity exercising it. Unlike some of the previous PRs on this, this is the minimal migration that would land this without adding any tech debt. From previous offline discussions we'd like to scope these migrations to the bare minimum required to keep the patches small. See #2114 for a migration combined with a refactoring. That said there are still some minor changes to prevent this from adding additional tech debt, going a little beyond #2118: - `#onMethodCall` has been split up into its own `MethodCallHandlerImpl` class to try and keep `CameraPlugin` from mixing concerns. Before the migration, _all_ it was doing was responding to method calls. Now there's a significant amount of logic just based around responding to lifecycle events. It still seemed better to split that into a separate file than to try and manage that logic all in one place. Other than the above refactor, the original logic is untouched.
Contributor
|
Closing since the camera plugin has been migrated. |
mormih
pushed a commit
to mormih/plugins
that referenced
this pull request
Nov 17, 2019
Migrates the camera plugin to the new embedding and adds an example activity exercising it. Unlike some of the previous PRs on this, this is the minimal migration that would land this without adding any tech debt. From previous offline discussions we'd like to scope these migrations to the bare minimum required to keep the patches small. See flutter#2114 for a migration combined with a refactoring. That said there are still some minor changes to prevent this from adding additional tech debt, going a little beyond flutter#2118: - `#onMethodCall` has been split up into its own `MethodCallHandlerImpl` class to try and keep `CameraPlugin` from mixing concerns. Before the migration, _all_ it was doing was responding to method calls. Now there's a significant amount of logic just based around responding to lifecycle events. It still seemed better to split that into a separate file than to try and manage that logic all in one place. Other than the above refactor, the original logic is untouched.
sungmin-park
pushed a commit
to sungmin-park/flutter-plugins
that referenced
this pull request
Dec 17, 2019
Migrates the camera plugin to the new embedding and adds an example activity exercising it. Unlike some of the previous PRs on this, this is the minimal migration that would land this without adding any tech debt. From previous offline discussions we'd like to scope these migrations to the bare minimum required to keep the patches small. See flutter#2114 for a migration combined with a refactoring. That said there are still some minor changes to prevent this from adding additional tech debt, going a little beyond flutter#2118: - `#onMethodCall` has been split up into its own `MethodCallHandlerImpl` class to try and keep `CameraPlugin` from mixing concerns. Before the migration, _all_ it was doing was responding to method calls. Now there's a significant amount of logic just based around responding to lifecycle events. It still seemed better to split that into a separate file than to try and manage that logic all in one place. Other than the above refactor, the original logic is untouched.
Akachu
pushed a commit
to Akachu/flutter_camera
that referenced
this pull request
Apr 27, 2020
Migrates the camera plugin to the new embedding and adds an example activity exercising it. Unlike some of the previous PRs on this, this is the minimal migration that would land this without adding any tech debt. From previous offline discussions we'd like to scope these migrations to the bare minimum required to keep the patches small. See flutter#2114 for a migration combined with a refactoring. That said there are still some minor changes to prevent this from adding additional tech debt, going a little beyond flutter#2118: - `#onMethodCall` has been split up into its own `MethodCallHandlerImpl` class to try and keep `CameraPlugin` from mixing concerns. Before the migration, _all_ it was doing was responding to method calls. Now there's a significant amount of logic just based around responding to lifecycle events. It still seemed better to split that into a separate file than to try and manage that logic all in one place. Other than the above refactor, the original logic is untouched.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An example of the simplest possible migration of the camera plugin to the new embedding with maximum code sharing.
The code quality in this PR is atrocious and it's probably completely untestable on the JVM, but it's a reference for the ecosystem team.