Skip to content

Conversation

@geriux
Copy link
Contributor

@geriux geriux commented Feb 15, 2023

This PR aims to block Buildkite steps to run on the Dependabot PRs we have for the submodules, Jetpack, Block experiments, and Gutenberg.

We are constantly seeing failed CI checks due to a pre-existing build on S3. Until there's a solution for that, I propose blocking the steps so we don't have failures on those PRs.

There will be a button to manually run these steps on these branches:

Screen Shot 2023-02-15 at 16 39 25

There's no need to have Android builds uploaded for these PRs since as far as I know we never make main apps PRs targeting those PRs.

To test

CI checks should pass and Builtkite steps should be blocked.

This other PR #5477 that has the same changes but with a different branch name should run all Builtkite steps without manually approving the steps.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@geriux geriux added the Testing Anything related to automated tests label Feb 15, 2023
@peril-wordpress-mobile
Copy link

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@geriux geriux marked this pull request as ready for review February 15, 2023 15:47
Copy link
Contributor

@fluiddot fluiddot left a comment

Choose a reason for hiding this comment

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

LGTM 🎊 !

As you mentioned in the PR's description is very unlikely that we merge a Dependabot PR, as we mainly use it for spotting breakages from the submodules. My only concern is, based on the PR's job status, how clear is that those jobs are blocked and required to be manually triggered. I don't see it as a critical issue as once you go to the Buildkite job you see the button but I understand that for someone without this context, it might be a bit confusing the first time.

@geriux
Copy link
Contributor Author

geriux commented Feb 16, 2023

My only concern is, based on the PR's job status, how clear is that those jobs are blocked and required to be manually triggered. I don't see it as a critical issue as once you go to the Buildkite job you see the button but I understand that for someone without this context, it might be a bit confusing the first time.

Yeah that's a valid concern, I thought about making those check statuses optional but I'd be worried about people merging with those checks failing or enabling auto merging and accidentally breaking something. Let's merge and see if it becomes an issue in the day-to-day workflows.

For this PR for example, I need to run the tests so I can merge 😄 so let's see.

Thank you for the review!

@geriux geriux merged commit dce71c2 into trunk Feb 16, 2023
@geriux geriux deleted the dependabot/submodules/test-skipping-branches branch February 16, 2023 12:27
@geriux geriux added this to the 1.90.0 (21.9) milestone Feb 16, 2023
@dcalhoun dcalhoun mentioned this pull request Mar 2, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Testing Anything related to automated tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants