Skip to content

Conversation

@chunhtai
Copy link
Contributor

@chunhtai chunhtai commented Jul 20, 2023

Since applink validation tool is going to be a static tool, It won't have access to vmservices.

flutter.dev/go/static-tooling-in-devtools

I remove the vm services and also update the deeplink task to also include path pattern and custom scheme
http://go/android-applink-apis (internal only)

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 Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • 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.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 20, 2023
@chunhtai chunhtai force-pushed the android-deeplinks branch from 0c67612 to d50d21b Compare July 31, 2023 20:12
@chunhtai chunhtai changed the title Removes android applinks related vm services and updates print deepli… Removes android applinks related vm services and updates gradle task to dump file instead Jul 31, 2023
@chunhtai
Copy link
Contributor Author

Hi @christopherfujino Can you take a look first before I add test for analyzer?

@chunhtai chunhtai force-pushed the android-deeplinks branch 2 times, most recently from 7a07b74 to 3e12a20 Compare August 1, 2023 16:16
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 someone adds another arg after this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The addAndroidAnalysis adds a separator, so all the other args after this method will be group into one section unless they add a new separator

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Can you just inline those addFlag() invocations here then? That way the presentation is more obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do, If the way this pr expanding the flutter analyze looks good to you, I will go ahead and add test and make this pr ready for review

Copy link
Contributor

Choose a reason for hiding this comment

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

The overall approach LGTM. I think we still need some discussion around the exact flag names and help text, but that shouldn't block writing tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added test

auto-submit bot pushed a commit that referenced this pull request Aug 18, 2023
1. Remove vm service registration
2. combine print<variant>ApplicationId and print<variant>AppLinkDomain into one task dump<variant>AppLinkSettings, which dump all the data in a json file

The deeplink validation tool will be a static app in devtool instead of regular app. A Static app doesn't require a running app; therefore, we can't call these API through vmservices. I decided to convert these API into flutter analyzer command, which will be done in a separate PR #131009.

The reason these print tasks are converted into file dumps is to reduce the amount of data encoding and decoding. Instead of passing data through stdout, the devtool can read the files generated by gradle tasks instead.
@chunhtai chunhtai changed the title Removes android applinks related vm services and updates gradle task to dump file instead Add android analyzer commands for applinks Aug 21, 2023
@chunhtai chunhtai marked this pull request as ready for review August 21, 2023 18:35
Copy link
Contributor

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

LGTM

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 23, 2023
@auto-submit auto-submit bot merged commit 4930613 into flutter:master Aug 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 24, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 24, 2023
flutter/flutter@bd836cc...deeb811

2023-08-24 [email protected] Reverts "Roll Flutter Engine from aa98a9d2e86f to 3b92bb6eda19 (4 revisions) (#133224)" (flutter/flutter#133255)
2023-08-24 [email protected] Roll Flutter Engine from aa98a9d2e86f to 3b92bb6eda19 (4 revisions) (flutter/flutter#133224)
2023-08-24 [email protected] Roll Flutter Engine from 67e8b825cc91 to aa98a9d2e86f (4 revisions) (flutter/flutter#133220)
2023-08-24 [email protected] Roll Flutter Engine from 27d75f6221d2 to 67e8b825cc91 (1 revision) (flutter/flutter#133214)
2023-08-23 [email protected] Remove deprecated PlatformViewsService.synchronizeToNativeViewHierarchy (flutter/flutter#133175)
2023-08-23 [email protected] Roll Flutter Engine from 58dc868c26cb to 27d75f6221d2 (14 revisions) (flutter/flutter#133211)
2023-08-23 [email protected] Remove deprecated MaterialButtonWithIconMixin (flutter/flutter#133173)
2023-08-23 [email protected] Enable ChangeNotifier clients to dispatch event of object creation in constructor. (flutter/flutter#133060)
2023-08-23 [email protected] Enable literal_only_boolean_expressions (flutter/flutter#133186)
2023-08-23 [email protected] Add android analyzer commands for applinks (flutter/flutter#131009)
2023-08-23 [email protected] Bump memory usage in gradle for platform views (flutter/flutter#133155)
2023-08-23 [email protected] [flutter_tools] Fix legacy version file not being ensured (flutter/flutter#133097)
2023-08-23 [email protected] Remove deprecated *TestValues from TestWindow (flutter/flutter#131098)
2023-08-23 [email protected] Roll Flutter Engine from ac352ba95f43 to 58dc868c26cb (2 revisions) (flutter/flutter#133165)
2023-08-23 [email protected] [web] Remove unnecessary lint ignore (flutter/flutter#133164)
2023-08-23 [email protected] Add `--experimental-wasm-type-reflection` and support newer emscripten builds. (flutter/flutter#133084)
2023-08-23 [email protected] Roll Flutter Engine from 72a06427bd37 to ac352ba95f43 (2 revisions) (flutter/flutter#133158)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants