[quick_actions] convert to pigeon by tarrinneal · Pull Request #5159 · flutter/packages

tarrinneal · 2023-10-17T04:15:18Z

Converts from direct use of method channels to using Pigeon.

Part of flutter/flutter#117844
fixesflutter/flutter#117913
fixesflutter/flutter#117112

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

stuartmorgan-g

LGTM with nits. Thanks for doing that Pigeon PR! The tests are much cleaner now, and that was definitely a good example of why it's valuable for Pigeon development for us to adopt it :)

tarrinneal · 2023-10-20T17:30:40Z

@hellohuanlin I would love to get an approval from you, since I'm still not 100% on Swift etiquette.

hellohuanlin · 2023-10-20T18:36:28Z

 }
 }
+
+ static func parseShortcutItems(_ items: [ShortcutItemMessage]) -> [UIApplicationShortcutItem] {


this doesn't seem to belong to a static member of QuickActionsPlugin. I suggest 2 alternatives -
make them private non-static members of QuickActionsPlugin (so they become implementation details QuickActionsPlugin).
A more swifty way I would do is to create class extension of either UIApplicationShortcutItem or ShortcutItemMessage, outside of QuickActionsPlugin

Sounds good, thanks for the feedback!

Oh also avoid the naming "parse" since it's not parsing anymore. Something like:
extension ShortcutItemMessage { var item: UIApplicationShortcutItem { return ... } }

I don't think we need the tests that I was using this for any longer, since this is now an implementation detail and my understanding is that the swifty thing to do is consider the converter tested, since it is used in code that is already tested.

Yes if it's implementation detail, it can only be tested via non-private members. You can delete those tests if it's already covered elsewhere

flutter/packages@be915be...4bf5114 2023-10-23 PROGrand@users.noreply.github.com [camera] CameraPlatform.createCameraWithSettings (flutter/packages#3615) 2023-10-23 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.3 to 2.22.4 (flutter/packages#5201) 2023-10-22 engine-flutter-autoroll@skia.org Roll Flutter from 6f4850d to 823e083 (3 revisions) (flutter/packages#5198) 2023-10-21 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.0 to 4.1.1 (flutter/packages#5167) 2023-10-21 engine-flutter-autoroll@skia.org Roll Flutter from 0883cb2 to 6f4850d (24 revisions) (flutter/packages#5196) 2023-10-21 50643541+Mairramer@users.noreply.github.com [ios_platform_images] migrate objC to swift (flutter/packages#4847) 2023-10-20 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/example/android/app (flutter/packages#5149) 2023-10-20 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#5150) 2023-10-20 tarrinneal@gmail.com [quick_actions] convert to pigeon (flutter/packages#5159) 2023-10-20 737941+loic-sharma@users.noreply.github.com [ci] Add build-only Windows Arm64 tests (flutter/packages#5142) 2023-10-20 43759233+kenzieschmoll@users.noreply.github.com Add '--no-tree-shake-icons' option to `BenchmarkServer` (flutter/packages#5186) 2023-10-20 engine-flutter-autoroll@skia.org Roll Flutter from c2bd2c1 to 0883cb2 (24 revisions) (flutter/packages#5192) 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-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

* main: (139 commits) Change firebase device tests from Android api 30 to 33 (flutter#5185) [url_launcher] Add an `inAppBrowserView` mode (flutter#5205) [ci] Remove device_type property from Windows Arm64 properties (flutter#5193) [url_launcher_web] Disallows launching "javascript:" URLs. (flutter#5180) [local_auth]: Bump androidx.fragment:fragment from 1.6.0 to 1.6.1 in /packages/local_auth/local_auth_android/android (flutter#4600) Roll Flutter from 823e083 to 5e8b5f4 (13 revisions) (flutter#5208) [tool] Add optional swift-format support (flutter#5204) [camera] CameraPlatform.createCameraWithSettings (flutter#3615) Bump github/codeql-action from 2.22.3 to 2.22.4 (flutter#5201) Roll Flutter from 6f4850d to 823e083 (3 revisions) (flutter#5198) Bump actions/checkout from 4.1.0 to 4.1.1 (flutter#5167) Roll Flutter from 0883cb2 to 6f4850d (24 revisions) (flutter#5196) [ios_platform_images] migrate objC to swift (flutter#4847) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/example/android/app (flutter#5149) [in_app_pur]: Bump org.json:json from 20230618 to 20231013 in /packages/in_app_purchase/in_app_purchase_android/android (flutter#5150) [quick_actions] convert to pigeon (flutter#5159) [ci] Add build-only Windows Arm64 tests (flutter#5142) Add '--no-tree-shake-icons' option to `BenchmarkServer` (flutter#5186) Roll Flutter from c2bd2c1 to 0883cb2 (24 revisions) (flutter#5192) [ci] Finalize migration to x64 specific Windows platform (flutter#5174) ...

Converts from direct use of method channels to using Pigeon. Part of flutter/flutter#117844fixesflutter/flutter#117913fixesflutter/flutter#117112

github-actionsBot added p: quick_actions platform-ios labels Oct 17, 2023

tarrinneal force-pushed the QA-pigeon branch 3 times, most recently from fda2e18 to 1cf1b5dCompare October 18, 2023 03:45

swift
409cd34

tarrinneal force-pushed the QA-pigeon branch from 1d40324 to 409cd34Compare October 18, 2023 10:32

tarrinneal requested a review from stuartmorgan-g October 18, 2023 10:32

doc
cbbd121

tarrinneal marked this pull request as ready for review October 18, 2023 12:09

tarrinneal requested a review from hellohuanlin as a code owner October 18, 2023 12:09

stuartmorgan-g mentioned this pull request Oct 18, 2023
[pigeon] Add a protocol for Flutter APIs in the Swift generator flutter/flutter#136811
Closed

stuartmorgan-g requested changes Oct 18, 2023
View reviewed changes

hellohuanlin reviewed Oct 18, 2023
View reviewed changes

hellohuanlin changed the title ~~Qa pigeon~~[quick_actions] convert to pigeonOct 18, 2023

tarrinneal mentioned this pull request Oct 19, 2023
[pigeon] add flutter api protocol #5181
Merged
11 tasks

tarrinneal added 3 commits October 19, 2023 08:20

Merge branch 'main' of github.com:flutter/packages into QA-pigeon
9eaaaf2

Merge branch 'main' of github.com:flutter/packages into QA-pigeon
f23b103

nits and test fixes
01599f2

tarrinneal commented Oct 20, 2023
View reviewed changes

Comment threadpackages/quick_actions/quick_actions_ios/ios/Classes/QuickActionsPlugin.swift Outdated

Merge branch 'main' of github.com:flutter/packages into QA-pigeon
975dcb2

tarrinneal requested review from hellohuanlin and stuartmorgan-g October 20, 2023 05:41

stuartmorgan-g approved these changes Oct 20, 2023
View reviewed changes

tarrinneal added 2 commits October 20, 2023 09:09

nits
2ca3980

delete file
110a311

hellohuanlin reviewed Oct 20, 2023
View reviewed changes

tarrinneal added 2 commits October 20, 2023 11:21

remove parser class
20796bb

method names
e2dc2b7

hellohuanlin reviewed Oct 20, 2023
View reviewed changes

non static
bfe3748

hellohuanlin approved these changes Oct 20, 2023
View reviewed changes

tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2023

tarrinneal added 2 commits October 20, 2023 12:06

message
f8bf4b2

selfies
249d3f8

auto-submitBot merged commit d1fcbab into flutter:mainOct 20, 2023

engine-flutter-autoroll mentioned this pull request Oct 23, 2023
Roll Packages from be915be92708 to 4bf51144c295 (12 revisions) flutter/flutter#137062
Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 23, 2023
d1fcbab[quick_actions] convert to pigeon (flutter/packages#5159)
0191534

reidbaker mentioned this pull request Jan 12, 2024
Dialog Debug #5881
Closed

reidbaker mentioned this pull request Jan 19, 2024
[in_app_purchase] Add play country code api #5941
Merged
12 tasks

reidbaker mentioned this pull request Feb 19, 2024
[in_app_purchase_android] Add UserChoiceBilling mode. #6162
Merged
16 tasks

Uh oh!

Conversation

tarrinneal commented Oct 17, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tarrinneal commented Oct 20, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hellohuanlinOct 20, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarrinnealOct 20, 2023

Choose a reason for hiding this comment

Uh oh!

hellohuanlinOct 20, 2023

Choose a reason for hiding this comment

Uh oh!

tarrinnealOct 20, 2023

Choose a reason for hiding this comment

Uh oh!

hellohuanlinOct 20, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tarrinneal commented Oct 17, 2023•
edited
Loading

hellohuanlinOct 20, 2023•
edited
Loading