woocommerce-android icon indicating copy to clipboard operation
woocommerce-android copied to clipboard

Bump Gradle wrapper to 8.8

Open wzieba opened this issue 1 year ago โ€ข 2 comments

Description

This PR bumps Gradle wrapper to 8.8.

Testing information

CI checks should be just fine.

  • [x] I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

wzieba avatar Jun 24 '24 11:06 wzieba

๐Ÿ“ฒ You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
PlatformโŒš๏ธ Wear OS
FlavorJalapeno
Build TypeDebug
Commite66afb98f2e716c32ff411176d711d411af62e75
Direct Downloadwoocommerce-wear-prototype-build-pr11778-e66afb9.apk

wpmobilebot avatar Jun 24 '24 11:06 wpmobilebot

๐Ÿ“ฒ You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform๐Ÿ“ฑ Mobile
FlavorJalapeno
Build TypeDebug
Commite66afb98f2e716c32ff411176d711d411af62e75
Direct Downloadwoocommerce-prototype-build-pr11778-e66afb9.apk

wpmobilebot avatar Jun 24 '24 11:06 wpmobilebot

1 Warning
:warning: This PR is assigned to the milestone 19.4. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by :no_entry_sign: Danger

dangermattic avatar Jul 03 '24 19:07 dangermattic

Btw, why are we doing this update now, I am just trying to understand whether there was a need for that, for security purposes maybe, or whether this was done purely to keep our tooling up-to-date, and maybe, to also have Gradle prepared for a future AGP update? ๐Ÿค”

It's a preparation for AGP update, but also a casual dependency update. I think that we were usually combining AGP and Gradle updates, but there's no need to not update Gradle separately and more often.

I issued another such update command and it gave me an updated gradlew and gradlew-wrapper.jar file. Can you try it again on your side and see if you also are getting these file to be re-updated, just like it happens on my side? ๐Ÿ™

I've doubled check and there's no difference for my gradlew and gradlew-wrapper.jar files. Also, I think "Validate Gradle Wrapper" should throw an error if these files were not precisely as they intend to be ๐Ÿค” . Could you please double check if there's still a diff when you run wrapper task?

wzieba avatar Jul 03 '24 20:07 wzieba

It's a preparation for AGP update, but also a casual dependency update. I think that we were usually combining AGP and Gradle updates, but there's no need to not update Gradle separately and more often.

I agree, maybe in the past, with some major updates, we had to do both, but unless we do have to do both, let's always do Gradle in isolation, and only then follow-up on AGP updates. ๐Ÿ’ฏ

I've doubled check and there's no difference for my gradlew and gradlew-wrapper.jar files. Also, I think "Validate Gradle Wrapper" should throw an error if these files were not precisely as they intend to be ๐Ÿค” . Could you please double check if there's still a diff when you run wrapper task?

Yea, I did double and triple check that, me running ./gradlew wrapper --gradle-version=8.8 ... will always produce a new diff. On the gradlew side it is just a comment change (can be ignored I guess), but on the gradlew-wrapper.jar side I am also seeing some code changes (can't be too much ignored). ๐Ÿค”

I wonder why I get a diff and you don't, this is so strange and unpredictable, and imagine, we both use a Mac... ๐Ÿคท


FYI: I don't want to block the merge so feel free to merge this anyway. ๐Ÿ˜Š

PS: Same for WPAndroid.

ParaskP7 avatar Jul 04 '24 07:07 ParaskP7

Yea, I did double and triple check that, me running ./gradlew wrapper --gradle-version=8.8 ... will always produce a new diff.

This is concerning ๐Ÿ˜• - could you please prepare a PR (or only branch) with the diff? I'd like to examine the difference.

wzieba avatar Jul 04 '24 12:07 wzieba

Sure thing @wzieba I just did that and pushed this commit here on this bump_gradle_wrapper_petros branch. ๐Ÿ‘

Let me know if that helps. ๐Ÿ™

ParaskP7 avatar Jul 04 '24 12:07 ParaskP7

So both versions are safe:

From this PR: image

From your commit image

But comparing this to https://gradle.org/release-checksums/, it looks like my update updated jar to version 8.6 ๐Ÿคจ The difference is that I used local gradle installed and I have it in version 8.6. But it shouldn't matter, as the task parameters point to 8.8.

I'll go ahead and cherry-pick your commit and merge this PR.

wzieba avatar Jul 04 '24 13:07 wzieba

...it looks like my update updated jar to version 8.6 ๐Ÿคจ

Oh, interesting, and I was wondering what was happening there, thanks for sharing! ๐Ÿ˜…

I'll go ahead and cherry-pick your commit and merge this PR.

๐Ÿš€

ParaskP7 avatar Jul 04 '24 13:07 ParaskP7