Repository navigation
[quick_actions] convert to pigeon - #5159
Conversation
fda2e18 to
1cf1b5d
Compare
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
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 :)
|
@hellohuanlin I would love to get an approval from you, since I'm still not 100% on Swift etiquette. |
| } | ||
| } | ||
|
|
||
| static func parseShortcutItems(_ items: [ShortcutItemMessage]) -> [UIApplicationShortcutItem] { |
There was a problem hiding this comment.
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 detailsQuickActionsPlugin). -
A more swifty way I would do is to create class extension of either
UIApplicationShortcutItemorShortcutItemMessage, outside ofQuickActionsPlugin
There was a problem hiding this comment.
Sounds good, thanks for the feedback!
There was a problem hiding this comment.
Oh also avoid the naming "parse" since it's not parsing anymore. Something like:
extension ShortcutItemMessage {
var item: UIApplicationShortcutItem {
return ...
}
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
tarrinneal
added
the
autosubmit
3 participants
Converts from direct use of method channels to using Pigeon.
Part of flutter/flutter#117844
fixes flutter/flutter#117913
fixes flutter/flutter#117112
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.