Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[in_app_purchase] update gradle tool to 4.1.0 #4079

Merged
cyanglaz merged 1 commit into from
Jun 22, 2021
Merged

[in_app_purchase] update gradle tool to 4.1.0 #4079

cyanglaz merged 1 commit into from
Jun 22, 2021

Conversation

@cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Jun 22, 2021

The gradle and gradle tool version have been upgraded in in_app_purchase_android e83a683, which caused a version mismatch in the in_app_purchase plugin

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 relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed 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.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@cyanglaz cyanglaz requested a review from LHLL as a code owner June 22, 2021 16:50
@google-cla google-cla bot added the cla: yes label Jun 22, 2021
@cyanglaz cyanglaz requested review from blasten and stuartmorgan-g and removed request for LHLL June 22, 2021 16:51
@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 22, 2021

If this mismatch is only an issue for this Android-specific test, wouldn't the better fix be to move the test into the Android plugin?

@cyanglaz
Copy link
Contributor Author

cyanglaz commented Jun 22, 2021 •
edited
Loading

@stuartmorgan there are actually no tests in in_app_purchase, the tests that are failing are in in_app_purchase_android.
I'm actually not sure why java tests in in_app_purchase_android would be executed with in_app_purchase/example

I'm trying to figure it out now. @blasten might know.

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 22, 2021

Do we have a weird configuration left over from flutter/flutter#81695 maybe?

@cyanglaz
Copy link
Contributor Author

cyanglaz commented Jun 22, 2021

I couldn't find anything that's suspicious. Seems like ./gradlew test tries to build test files in a dependency (in_app_purchase_android)

stuartmorgan-g
stuartmorgan-g approved these changes Jun 22, 2021
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.

Okay, LGTM to fix the tree then. Could you file a bug about this though? We don't want to be running all of our Android unit test twice as we federate more of the plugins.

@cyanglaz
Copy link
Contributor Author

cyanglaz commented Jun 22, 2021

@stuartmorgan Sure
I think there might be a worse issue regarding this tho. If our example app's ./gradlew test can fail because it uses a lower gradle version, any flutter app that uses in_app_purhcase_android would potentially face the same thing when they run ./gradlew test.

@cyanglaz cyanglaz merged commit 67885c5 into flutter:master Jun 22, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 22, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 22, 2021
@engine-flutter-autoroll
67885c5 [in_app_purchase] update gradle tool to 4.1.0 (flutter/plugin…
bc18d24 
…s#4079)
@cyanglaz cyanglaz deleted the iap_gradle branch June 22, 2021 18:38
@cyanglaz cyanglaz mentioned this pull request Jun 22, 2021
Closed
@blasten
Copy link

blasten commented Jun 22, 2021

The Gradle version set in a plugin only matters if you are running the plugin's java tests.

When an app is built, only one Gradle version is used (the one set by the app). (The analogy I find is, it'd would be like using two or more different Dart SDK versions to build a single app).

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 22, 2021

@blasten But why is the app-facing package's example app building the tests from the Android implementation package?

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented Jun 22, 2021

I didn't see the issue link before posting, we can continue the discussion there.

amantoux pushed a commit to amantoux/plugins that referenced this pull request Jul 10, 2021
@amantoux
[in_app_purchase] update gradle tool to 4.1.0 (flutter#4079)
a7e247a
@TahaTesser TahaTesser mentioned this pull request Aug 22, 2021
Closed
12 tasks
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@fotiDim
[in_app_purchase] update gradle tool to 4.1.0 (flutter#4079)
41e1ec4
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@blasten blasten Awaiting requested review from blasten

1 more reviewer

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

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes p: in_app_purchase platform-android

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cyanglaz @stuartmorgan-g @blasten