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

Conversation

@matthew-carroll
Copy link
Contributor

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.

…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.

dependencies {
// TODO: remove these dependencies when engine POM is working correctly.
implementation 'androidx.lifecycle:lifecycle-runtime:2.1.0'
Copy link
Contributor

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

Copy link

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

@bparrishMines bparrishMines changed the title Simplest camera plugin migration [camera] Simplest camera plugin migration Oct 4, 2019
@mklim mklim mentioned this pull request Oct 8, 2019
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.
@mklim
Copy link
Contributor

mklim commented Oct 31, 2019

Closing since the camera plugin has been migrated.

@mklim mklim closed this Oct 31, 2019
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.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants