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

mergify: fix needs work labels if CI fails #15141

Merged
merged 1 commit into from
Oct 20, 2021

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Oct 14, 2021

Summary of changes

We have a clash with needs work and needs CI labels. If CI fails, we need to be in CI stage. Check for needs: CI label and apply work only if it was CI.

Otherwise we would be in the loop (we had couple of PRs with 400 messages from mergify). Because we did not check for previous stage (like this fix is doing), we had PRs where both states were true (apply needs work and apply needs CI labels - remove any other label). If we are more strict, and apply needs work only if CI fails and it is in CI stage, we should avoid having this infinite loops.

Impact of changes

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[x] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


We have clash with needs work and needs CI labels. If CI fails, we need to be in CI stage. Check for needs: CI label and apply work only if it was CI.

Otherwise we would be in the loop.
@0xc0170 0xc0170 requested a review from adbridge October 14, 2021 14:01
@ciarmcom ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label Oct 14, 2021
@ciarmcom
Copy link
Member

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested a review from a team October 14, 2021 14:30
- -closed
actions:
label:
add: ['needs: work']
remove: ['needs: review','needs: CI']
remove: ['needs: CI']
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has the remove bit changed? If there is a needs:review doesn't that also need removing ?

Copy link
Contributor Author

@0xc0170 0xc0170 Oct 18, 2021

Choose a reason for hiding this comment

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

This stage is from CI to needs work, we should not touch review label as it was already removed in a stage before (review -> CI).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 62 should be more important for this fix. I removed review part of this fix as it does not make sense to be removed, or does it.

- "label!=mergify skip"
actions:
label:
add: ['needs: work']
remove: ['needs: review', 'needs: CI']
remove: ['needs: CI']
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same is above, these 2 stages here are for CI failures, nothing related to review.

i also added needs: CI needs to be already part of the PR.

@mergify mergify bot added needs: CI and removed needs: work labels Oct 20, 2021
@0xc0170
Copy link
Contributor Author

0xc0170 commented Oct 20, 2021

Ci started

@0xc0170 0xc0170 merged commit 54a4879 into ARMmbed:master Oct 20, 2021
@0xc0170 0xc0170 deleted the mergify-fix-review-labels branch October 20, 2021 13:52
@mergify mergify bot removed the ready for merge label Oct 20, 2021
@mbedmain mbedmain added release-version: 6.15.1 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants