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

Conversation

@matthew-carroll
Copy link
Contributor

New Plugin API PR5: Integrates plugin lifecycle control with FlutterFragment.

This should be the last of planned API work on the plugin API. The only remaining work should be the plugin shim that allows old plugins to work with the new embedding.

Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

@Override
public void attachToActivity(@NonNull Activity activity, @NonNull Lifecycle lifecycle) {
Log.d(TAG, "Attaching to an Activity.");
Log.d(TAG, "Attaching to an Activity."
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: We want to guard these. I think arguably this should be verbose instead of debug too.

../../../flutter/shell/platform/android/io/flutter/embedding/engine/FlutterEnginePluginRegistry.java:256: Error: The log call Log.d(...) should be conditional: surround with if (Log.isLoggable(...)) or if (BuildConfig.DEBUG) { ... } [LogConditional]
    Log.d(TAG, "Attaching to an Activity."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, changed the Log.d's to Log.v's. Do we have a style doc that says when to use one logging level vs another?

@matthew-carroll matthew-carroll force-pushed the 31589_new_plugin_api_pr5 branch from e96dd10 to fe1751b Compare May 30, 2019 03:23
@matthew-carroll matthew-carroll merged commit e8aa120 into flutter:master May 30, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 30, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 30, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request May 30, 2019
flutter/engine@fa9b5bd...7dd62d6

git log fa9b5bd..7dd62d6 --no-merges --oneline
7dd62d6 Roll src/third_party/skia 1013ecfb3421..859f7108a5af (19 commits) (flutter/engine#9136)
e8aa120 New Plugin API PR5: Integrates plugin lifecycle control with FlutterFragment. (flutter/engine#9083)
8b1199c Implemented Log proxy that only logs in BuildConfig.DEBUG (#25391). (flutter/engine#9122)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
huqiuser pushed a commit to huqiuser/engine that referenced this pull request Jun 12, 2019
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.

3 participants