Add Windows to the platform_channels example #106754
Conversation
flutter-dashboard
bot
added
d: examples
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@cbracken @gspencergoog Do we not have any devicelab Windows tests? I noticed that Ideally we'd be running the driver tests for all the examples on Windows, but I didn't see any place we already have that set up. |
|
Consider adding a note to the example's README about Windows support: https://github.com/flutter/flutter/tree/master/examples/platform_channel#example-of-calling-platform-services-from-flutter |
| @@ -3968,6 +3968,20 @@ targets: | |||
| - bin/** | |||
| - .ci.yaml | |||
|
|
|||
| - name: Windows platform_channel_sample_test_windows | |||
There was a problem hiding this comment.
@keyonghan Does this look right? As usual with .ci.yaml changes I don't do it often enough to remember how to manually test this in advance, and since it's bringup it won't be tested here.
There was a problem hiding this comment.
The ci config looks good to me.
You may want to add the test ownership entry in https://github.com/flutter/flutter/blob/master/TESTOWNERS#L218 by following:
# Windows platform_channel_sample_test_windows
<path-to-test> @<owner> @<team>
There was a problem hiding this comment.
Done. I gave it to @cbracken since desktop seemed like the best fit for it, but we could adjust it to me+plugins later if it turns out to be problematic.
| @@ -44,10 +44,11 @@ TaskFunction createExternalUiIntegrationTest() { | |||
| ); | |||
| } | |||
|
|
|||
| TaskFunction createPlatformChannelSampleTest() { | |||
| TaskFunction createPlatformChannelSampleTest({String? deviceIdOverride}) { | |||
There was a problem hiding this comment.
This and the change below follow the pattern from #45264
I think I figured out setting this up 🤞🏻. Added to the PR. |
Done. |
|
While fixing the missing licenses, I noticed that it was only flagging the .h files, not the .cpp files, so I fixed the script that checks this to check cpp files (we were only getting .h because it was configured for Obj-C already). And then fixed the missing licenses in other Windows examples. |
stuartmorgan
added
the
autosubmit
This adds a Windows section to the existing iOS and Android section, following the same structure as those examples, and using the code from flutter/flutter#106754 Part of flutter/flutter#79204
This adds a Windows section to the existing iOS and Android section, following the same structure as those examples, and using the code from flutter/flutter#106754 Part of flutter/flutter#79204
Adds Windows support to the
platform_channelsexample, implementing battery querying and charge state listening/reporting.This is mostly
flutter createboilerplate; see individual commits for actual changes.Also fixes the license check script to check
.cppfiles, and fixes the new violations that found.Part of #79204
Pre-launch Checklist
///).