-
Notifications
You must be signed in to change notification settings - Fork 6k
Make shell/platform/android IDE-friendly, and add documentation
#49612
Conversation
|
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 "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use 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. |
|
Only failure is the licensing bot, which I'll fix up before submitting. |
|
/cc @johnmccutchan as well. |
|
@dnfield and/or @jason-simmons - I'd love a quick sniff-test that none of the changes break your mental model. Thanks! |
jason-simmons
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
shell/platform/android/build.gradle
Outdated
| dependencies { | ||
| implementation 'androidx.annotation:annotation-jvm:1.7.1' | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the empty line here
|
auto label is removed for flutter/engine/49612, due to - The status or check suite Linux linux_license has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
auto label is removed for flutter/engine/49612, due to - The status or check suite Linux Web Framework tests has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…utter#49612) Closes flutter/flutter#141043. Nothing in this change should impact: - End-user apps - Testing and/or CI I've added what I believe is the minimum required to get a reasonable Android Studio experience with the `shell/platform/android` folder. More could definitely be done, but this unblocks me (i.e. flutter/flutter#139702) and adds instructions for the next person. _I'm open to suggestions on how to improve this further, but unless they are critical, I'd rather land this as-is and review your PRs tweaking my documentation further (I have limited time to work exclusively on docs)._ --- Example after this: 
…141215) flutter/engine@693af0c...8c1501f 2024-01-09 [email protected] Make `shell/platform/android` IDE-friendly, and add documentation (flutter/engine#49612) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll 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 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
| } | ||
|
|
||
| dependencies { | ||
| implementation 'androidx.annotation:annotation-jvm:1.7.1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late to the party here, but did you add this to resolve androidx.annotation... imports? I tried following the README from this PR, and it works for all imports except the androidx ones. Also, this version should probably match the one in files.json, right?
Closes flutter/flutter#141043.
Nothing in this change should impact:
I've added what I believe is the minimum required to get a reasonable Android Studio experience with the
shell/platform/androidfolder. More could definitely be done, but this unblocks me (i.e. flutter/flutter#139702) and adds instructions for the next person.I'm open to suggestions on how to improve this further, but unless they are critical, I'd rather land this as-is and review your PRs tweaking my documentation further (I have limited time to work exclusively on docs).
Example after this: