-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Remove block and line comments when detecting '.flutter-plugins' in settings.gradle(.kts) #155488
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…from settings.gradle file
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). 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. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
bkonyi
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.
Sorry for the delay on this review. LGTM!
bkonyi
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.
Actually, it looks like we're missing some imports in gradle_test.dart. Can you add those, fix the trailing space on line 893, and run the test to make sure it's passing?
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
To pass the PR checks, I have made several commits, resulting in a lengthy commit history. Therefore, I have performed a rebase and force-pushed the final code submission. |
Yes, I've made the changes as suggested. I've moved the test code from the general.shard to the integration.shard and taken care of it. |
|
Thanks @goosetapa! Looks good to me! |
…egacy_flutter_plugins_strings_in_comments_test.dart Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
andrewkolos
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 :)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
Roll Flutter from c8510f2 to ed553d1 (48 revisions) flutter/flutter@c8510f2...ed553d1 2024-11-13 matanlurey@users.noreply.github.com Avoid using platform `ProcessInfo.maxRss` in test. (flutter/flutter#158526) 2024-11-13 engine-flutter-autoroll@skia.org Roll Packages from 72356fd to 26e123a (19 revisions) (flutter/flutter#158626) 2024-11-13 matanlurey@users.noreply.github.com Move `dart pub deps` call to `<Pub>.deps` and use it accordingly (flutter/flutter#158524) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 05c14d8b4cd7 to db3e5af2ca22 (1 revision) (flutter/flutter#158553) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from ef760d6e1f13 to 05c14d8b4cd7 (3 revisions) (flutter/flutter#158551) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 08348c9eebcc to ef760d6e1f13 (1 revision) (flutter/flutter#158545) 2024-11-13 fluttergithubbot@gmail.com Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be flaky (flutter/flutter#158242) 2024-11-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 877abb9ad6ff to 08348c9eebcc (8 revisions) (flutter/flutter#158541) 2024-11-13 matanlurey@users.noreply.github.com Allow `devDependencies` to be omitted and not cause a tool crash. (flutter/flutter#158518) 2024-11-13 matanlurey@users.noreply.github.com Explain how to use `flutter channel`. (flutter/flutter#158533) 2024-11-13 magder@google.com Clean up dependabot config, add github-action group (flutter/flutter#158408) 2024-11-12 reidbaker@google.com Update test to include more complete instructions for how to run tests locally, add example to andoid 11 tests as well (flutter/flutter#158528) 2024-11-12 christopherfujino@gmail.com force Linux plugin_test to run on Ubuntu 20.04 (flutter/flutter#158529) 2024-11-12 jhy03261997@gmail.com Support materialTapTargetSize in PopupMenuButton (flutter/flutter#158357) 2024-11-12 737941+loic-sharma@users.noreply.github.com [SwiftPM] Update .flutter-plugin-dependencies format (flutter/flutter#158138) 2024-11-12 andrewrkolos@gmail.com add filesystem error handling to `systemTempDirectory` (flutter/flutter#158481) 2024-11-12 jiten.pateljp2001@gmail.com Made Cupertino dialog more like a native dialog in dark mode (flutter/flutter#157218) 2024-11-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from b0a4ca92c49e to 877abb9ad6ff (2 revisions) (flutter/flutter#158506) 2024-11-12 tessertaha@gmail.com Fix `NavigationBar` label style customization on the widget level (flutter/flutter#158510) 2024-11-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add test for `dynamic_content_color.0.dart` (#158309)" (flutter/flutter#158511) 2024-11-12 32538273+ValentinVignal@users.noreply.github.com Add test for `dynamic_content_color.0.dart` (flutter/flutter#158309) 2024-11-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from a672f971c659 to b0a4ca92c49e (2 revisions) (flutter/flutter#158488) 2024-11-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 35041f118744 to a672f971c659 (1 revision) (flutter/flutter#158487) 2024-11-12 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7b3eacd20eb6 to 35041f118744 (9 revisions) (flutter/flutter#158485) 2024-11-11 737941+loic-sharma@users.noreply.github.com [SwiftPM] Simplify logic that determines if CocoaPods is used (flutter/flutter#158409) 2024-11-11 tessertaha@gmail.com Fix Chip draws `InkWell.hoverColor` is drawn on top of the provided background color with `hovered` state (flutter/flutter#158454) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3cb6f4de89b6 to 7b3eacd20eb6 (1 revision) (flutter/flutter#158464) 2024-11-11 engine-flutter-autoroll@skia.org Roll Packages from bb5a258 to 72356fd (8 revisions) (flutter/flutter#158378) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from e9a44820f302 to 3cb6f4de89b6 (3 revisions) (flutter/flutter#158456) 2024-11-11 bkonyi@google.com Replace custom `RPCErrorCodes` with `RPCErrorKind` from `package:vm_service` (flutter/flutter#158379) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from d90e9f4718b8 to e9a44820f302 (1 revision) (flutter/flutter#158453) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 01c76e42c20f to d90e9f4718b8 (1 revision) (flutter/flutter#158443) 2024-11-11 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9b4c3b3d5518 to 01c76e42c20f (3 revisions) (flutter/flutter#158438) 2024-11-11 proddam@gmail.com Remove block and line comments when detecting '.flutter-plugins' in settings.gradle(.kts) (flutter/flutter#155488) 2024-11-11 nate.w5687@gmail.com Add `SafeArea` DartPad sample (flutter/flutter#158019) 2024-11-10 fluttergithubbot@gmail.com Marks Linux analyzer_benchmark to be flaky (flutter/flutter#158244) 2024-11-10 andrewrkolos@gmail.com remove `bringup` status for recently re-subsharded targets (flutter/flutter#158217) 2024-11-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 690cdfd09beb to 9b4c3b3d5518 (1 revision) (flutter/flutter#158418) 2024-11-09 fluttergithubbot@gmail.com Marks Mac_benchmark complex_layout_scroll_perf_macos__timeline_summary to be flaky (flutter/flutter#158252) 2024-11-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from ca6f5110d9d3 to 690cdfd09beb (1 revision) (flutter/flutter#158414) 2024-11-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 2f097cfd3d2d to ca6f5110d9d3 (3 revisions) (flutter/flutter#158411) 2024-11-09 engine-flutter-autoroll@skia.org Roll Flutter Engine from 54df0b8a4784 to 2f097cfd3d2d (1 revision) (flutter/flutter#158407) 2024-11-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from b7134d373ef8 to 54df0b8a4784 (2 revisions) (flutter/flutter#158405) 2024-11-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 6b77347edfc5 to b7134d373ef8 (3 revisions) (flutter/flutter#158402) 2024-11-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1b567e80386e to 6b77347edfc5 (4 revisions) (flutter/flutter#158398) 2024-11-08 engine-flutter-autoroll@skia.org Roll Flutter Engine from a08bd5a07c2a to 1b567e80386e (1 revision) (flutter/flutter#158393) ...
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
…ns' in settings.gradle(.kts) (flutter/flutter#155488)
PR Title:
Remove block and line comments when detecting
'.flutter-plugins'insettings.gradleDescription:
This PR modifies the
configureLegacyPluginEachProjectsfunction to remove block (/* ... */) and line (// ...) comments from thesettings.gradleorsettings.gradle.ktsfile content before checking for the presence of the'.flutter-plugins'string. This ensures that only uncommented, meaningful code is considered during the detection, preventing false positives when the string appears within comments.Why is this change necessary?
In some cases, the
'.flutter-plugins'string may be present inside comments in thesettings.gradlefile. The existing implementation does not account for this and may incorrectly detect the string even when it's commented out. This can lead to unintended behavior, such as configuring plugin projects when it is not necessary.By removing comments before performing the check, we prevent false positives and ensure that the detection logic is accurate, only acting when the
'.flutter-plugins'string is present in active code.Changes Made:
Added comment removal logic:
/* ... */) using the regular expression/(?s)\/\*.*?\*\//.(?s)flag enables dot-all mode, allowing.to match newline characters.// ...) using the regular expression/(?m)\/\/.*$.(?m)flag enables multi-line mode, so^and$match the start and end of each line.Updated the string detection:
'.flutter-plugins'is now performed on the uncommented content of thesettings.gradlefile.Issue Fixed:
Pre-launch Checklist
///).If you need any further assistance or have questions, feel free to reach out!
Links: