Skip to content

Conversation

@bartekpacia
Copy link
Member

@bartekpacia bartekpacia commented Jan 24, 2024

This PR fixes 2 small mistakes in FlutterExtension:

  • all fields must be public in order to be used in Gradle Kotlin DSL the same as in Gradle Groovy DSL
  • using logger instead of project.logger throws an error when executed

This PR re-adds a subset of changes from #141541 which broke the tree and has been reverted.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@bartekpacia bartekpacia requested a review from reidbaker January 24, 2024 16:53
@flutter-dashboard
Copy link

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.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jan 24, 2024
@bartekpacia bartekpacia changed the title flutter.groovy: make final fields also public flutter.groovy: update for Gradle Kotlin DSL compatibility Jan 24, 2024
@bartekpacia
Copy link
Member Author

Note to the potential "test-exemption" giver: this small PR introduces a subset of functionality that already landed in #141541 but then it broke the tree (unknown reasons - see thread).

I'd like to land this small PR first, see that nothing breaks in the tree, and then proceed with the rest of changes from #141541.

@bartekpacia
Copy link
Member Author

@reidbaker Could you check what's going on with Google Testing?

@reidbaker
Copy link
Contributor

overrode the google testing status looks like a back end infra issue

File buildGradleKts = new File(path, 'android' + File.separator + 'build.gradle.kts')
if (buildGradle.exists() && buildGradleKts.exists()) {
logger.error(
project.logger.error(
Copy link
Member Author

@bartekpacia bartekpacia Jan 24, 2024

Choose a reason for hiding this comment

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

this bug existed because code inside this if path is not exercised by any test

File settingsGradleKts = new File(project.projectDir.parentFile, "settings.gradle.kts")
if (settingsGradle.exists() && settingsGradleKts.exists()) {
logger.error(
project.logger.error(
Copy link
Member Author

@bartekpacia bartekpacia Jan 24, 2024

Choose a reason for hiding this comment

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

this bug existed because code inside this if path is not exercised by any test

@bartekpacia
Copy link
Member Author

Aight, I'm gonna merge this without an exemption. I'm okay if this gets reverted, then I'll double down to get this right haha

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?

@bartekpacia bartekpacia added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 26, 2024
@auto-submit auto-submit bot merged commit 370f40e into flutter:master Jan 26, 2024
@bartekpacia bartekpacia deleted the flutter_groovy_fix branch January 26, 2024 10:01
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 26, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 28, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 29, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants