Skip to content
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

Gradle Wrapper Validation #17480

Merged
merged 4 commits into from
Nov 24, 2022
Merged

Gradle Wrapper Validation #17480

merged 4 commits into from
Nov 24, 2022

Conversation

oguzkocer
Copy link
Contributor

@oguzkocer oguzkocer commented Nov 18, 2022

In Automattic/a8c-ci-toolkit-buildkite-plugin#34 we introduced our own script to validate the Gradle Wrapper which uses the official documentation for its implementation. This PR uses that script to add a prerequisite step to validate the Gradle Wrapper in all 3 pipelines.

The main reason we decided to add our own script as opposed to using the official Github Action was to be able to validate the Gradle Wrapper before running any of the Buildkite steps. As discussed in that PR, there are some edge cases our script doesn't cover. Furthermore, because it's a custom implementation, there is always a chance that it gets outdated. Therefore, we are adding the official Github Action alongside our own script.

After this PR is merged, I suggest we add the Github Action as a required step for trunk branch. The Buildkite step is already a prerequisite for the rest of the steps, so I don't think we need to add it as a required check, but having the Github Action as a required check should give us some extra peace of mind. Github only takes ~20s to validate the wrapper, so it shouldn't block developers unless there is an issue with it.

To test:

  • Verify that the Gradle Wrapper Validation is added as a prerequisite step to the Buildkite pipeline.

Screen Shot 2022-11-23 at 11 13 29 PM

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 18, 2022

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17480-3f049a1.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commit3f049a1
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 18, 2022

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17480-3f049a1.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commit3f049a1
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@oguzkocer oguzkocer marked this pull request as ready for review November 24, 2022 04:19
@oguzkocer oguzkocer requested a review from a team November 24, 2022 04:19
@oguzkocer oguzkocer added this to the 21.3 milestone Nov 24, 2022
@oguzkocer oguzkocer changed the title Adds Gradle Wrapper Validation to Buildkite Adds Gradle Wrapper Validation Nov 24, 2022
@oguzkocer oguzkocer changed the title Adds Gradle Wrapper Validation Gradle Wrapper Validation Nov 24, 2022
@ParaskP7 ParaskP7 self-assigned this Nov 24, 2022
Copy link
Contributor

@ParaskP7 ParaskP7 left a comment

Choose a reason for hiding this comment

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

👋 @oguzkocer !

Great work on the adding the Gradle Wrapper Validation checks in WPAndroid! 🥇 ❤️ 🚀

I've reviewed and tested the PR as per the description and everything LGTM:

  • Configuration Changes ✅
  • Buildkite ✅
  • GitHub Actions ✅

After this PR is merged, I suggest we add the Github Action as a required step for trunk branch.

I totally agree! 💯

@oguzkocer oguzkocer merged commit 2edb3e0 into trunk Nov 24, 2022
@oguzkocer oguzkocer deleted the add/gradle-wrapper-validation branch November 24, 2022 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants