Skip to content

[all_packages] Enforce property assignment for compile sdk over method assignment #8897

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Apr 3, 2025
Merged

[all_packages] Enforce property assignment for compile sdk over method assignment #8897

merged 9 commits into from
Apr 3, 2025

Conversation

reidbaker
Copy link
Contributor

No issue I just noticed that we were almost consistent in our approach to compileSdk.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or I have commented below to indicate which [version change exemption] this PR falls under[^1].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or I have commented below to indicate which [CHANGELOG exemption] this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@reidbaker reidbaker marked this pull request as ready for review March 25, 2025 17:06
stuartmorgan-g
stuartmorgan-g reviewed Mar 25, 2025
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tool changes LGTM, but I'm still seeing ~40 violations of this locally.

script/tool/lib/src/gradle_check_command.dart Outdated
if (methodAssignmentLine == null) {
printError('${indentation}No compileSdk or compileSdkVersion found.');
} else {
printError('${indentation}No compileSdk = found. Please use property assignment.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about No "compileSdk =" found to make this easier to parse if you don't already know what the issue is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Mar 25, 2025

autosubmit label was removed for flutter/packages/8897, because - The status or check suite Linux_web web_dart_unit_test_wasm_shard_2 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
chunhtai
chunhtai approved these changes Mar 25, 2025
View reviewed changes
Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@reidbaker
Copy link
Contributor Author

Failures appear unrelated. Hopefully an updated branch will fix the issue.

@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 25, 2025
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Mar 25, 2025

autosubmit label was removed for flutter/packages/8897, because - The status or check suite Linux_web web_dart_unit_test_wasm_shard_2 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@reidbaker
Copy link
Contributor Author

Blocked on flutter/flutter#165664

@ditman
Copy link
Member

ditman commented Mar 27, 2025

Clicked "Update branch" because web tests seem to be fixed in master after @stuartmorgan's fix.

@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2025
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Mar 27, 2025

autosubmit label was removed for flutter/packages/8897, because - The status or check suite Mac_arm64 ios_platform_tests_shard_2 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan-g
Copy link
Contributor

autosubmit label was removed for flutter/packages/8897, because - The status or check suite Mac_arm64 ios_platform_tests_shard_2 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

I'm seeing this same issue on a roller PR. I'm wondering if it's OOB failure from a maps SDK or server change.

@ditman
Copy link
Member

ditman commented Mar 28, 2025

Looking at the build history for the suite, it seems this was the first build broken like this was on: 2025-03-25 @ 09:55:

Associated to this commit:

  • 347d39a (the branch name according to github was "xcode_16_test" 🔮)

@stuartmorgan-g
Copy link
Contributor

Once #8967 lands, merging in main should unblock this.

@matanlurey matanlurey removed their request for review April 2, 2025 15:30
@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 3, 2025
@auto-submit auto-submit bot merged commit 4a36dc6 into flutter:main Apr 3, 2025
82 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 3, 2025
@engine-flutter-autoroll
4a36dc63f [all_packages] Enforce property assignment for compile sdk …
4404e4e 
…over method assignment (flutter/packages#8897)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 3, 2025
Merged
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Apr 3, 2025
@engine-flutter-autoroll
Roll Packages from 07496eb03035 to 4a36dc63f719 (3 revisions) (#166542)
02f13c3 
flutter/packages@07496eb...4a36dc6

2025-04-03 1063596+reidbaker@users.noreply.github.com [all_packages]
Enforce property assignment for compile sdk over method assignment
(flutter/packages#8897)
2025-04-03 stuartmorgan@google.com [google_maps_flutter] Fix iOS info
window regression (flutter/packages#8939)
2025-04-02 engine-flutter-autoroll@skia.org Roll Flutter from
05b5e79 to a0b1b32 (37 revisions) (flutter/packages#8985)

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 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
masterromuald pushed a commit to masterromuald/packages that referenced this pull request Apr 3, 2025
@reidbaker @masterromuald
[all_packages] Enforce property assignment for compile sdk over metho…
35a588b 
…d assignment (flutter#8897)

No issue I just noticed that we were almost consistent in our approach to compileSdk.
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
@reidbaker @androidseb
[all_packages] Enforce property assignment for compile sdk over metho…
744acd8 
…d assignment (flutter#8897)

No issue I just noticed that we were almost consistent in our approach to compileSdk.
zhangyuang pushed a commit to zhangyuang/flutter-fork that referenced this pull request Jun 9, 2025
@engine-flutter-autoroll
Roll Packages from 07496eb03035 to 4a36dc63f719 (3 revisions) (flutte…
85a519a 
…r#166542)

flutter/packages@07496eb...4a36dc6

2025-04-03 1063596+reidbaker@users.noreply.github.com [all_packages]
Enforce property assignment for compile sdk over method assignment
(flutter/packages#8897)
2025-04-03 stuartmorgan@google.com [google_maps_flutter] Fix iOS info
window regression (flutter/packages#8939)
2025-04-02 engine-flutter-autoroll@skia.org Roll Flutter from
05b5e79 to a0b1b32 (37 revisions) (flutter/packages#8985)

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 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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
@reidbaker @FMorschel
[all_packages] Enforce property assignment for compile sdk over metho…
71d6ba0 
…d assignment (flutter#8897)

No issue I just noticed that we were almost consistent in our approach to compileSdk.
Ortes pushed a commit to Ortes/packages that referenced this pull request Jun 25, 2025
@reidbaker @Ortes
[all_packages] Enforce property assignment for compile sdk over metho…
fc2cfd5 
…d assignment (flutter#8897)

No issue I just noticed that we were almost consistent in our approach to compileSdk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@chunhtai chunhtai chunhtai approved these changes

@tarrinneal tarrinneal tarrinneal approved these changes

@jesswrd jesswrd jesswrd approved these changes

@bparrishMines bparrishMines Awaiting requested review from bparrishMines bparrishMines is a code owner

@camsim99 camsim99 Awaiting requested review from camsim99 camsim99 is a code owner

@gmackall gmackall Awaiting requested review from gmackall gmackall is a code owner

@Piinks Piinks Awaiting requested review from Piinks Piinks is a code owner

@Hixie Hixie Awaiting requested review from Hixie Hixie is a code owner

@gspencergoog gspencergoog Awaiting requested review from gspencergoog

@domesticmouse domesticmouse Awaiting requested review from domesticmouse

@ditman ditman Awaiting requested review from ditman ditman is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera p: espresso p: extension_google_sign_in_as_googleapis_auth p: file_selector p: flutter_adaptive_scaffold p: flutter_image p: flutter_markdown p: flutter_plugin_android_lifecycle p: go_router p: google_maps_flutter p: google_sign_in p: image_picker p: in_app_purchase p: interactive_media_ads Plugin for IMA SDK p: local_auth p: palette_generator p: path_provider p: pigeon p: quick_actions p: rfw Remote Flutter Widgets p: shared_preferences p: url_launcher p: video_player p: webview_flutter platform-android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@reidbaker @ditman @stuartmorgan-g @chunhtai @tarrinneal @jesswrd