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

ci: support for "skip-ci" github label in order to reduce what to run #711

Closed
wants to merge 2 commits into from

Conversation

v1v
Copy link
Member

@v1v v1v commented Mar 1, 2022

What

skip-ci label will only run the linting stage and then will stop doing anything else.

Why

See the below use case:

As a developer, I don't want to run CI because I'm sure is going to fail and will just waste CI cycles

Issue

Similar to elastic/beats#21377

UX

See the below comment that points to the reason for the failure:

image

@v1v v1v requested review from endorama and a team March 1, 2022 11:36
@v1v v1v self-assigned this Mar 1, 2022
@v1v v1v added automation skip-ci Skip the build in the CI but linting labels Mar 1, 2022
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 1, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-01T11:51:14.969+0000

  • Duration: 20 min 23 sec

Test stats 🧪

Test Results
Failed 0
Passed 524
Skipped 1
Total 525

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@v1v v1v removed the skip-ci Skip the build in the CI but linting label Mar 1, 2022
@v1v
Copy link
Member Author

v1v commented Mar 1, 2022

/test

this will avoid the notification regarding the google storage
@@ -145,6 +153,10 @@ pipeline {
}
post {
always {
// IMPORTANT: notify a build status in order to avoid GitHub checks with a wrong status.
whenTrue(matchesPrLabel(label: 'skip-ci')) {
error 'Pull Request has been configured to be disabled when there is a skip-ci label match'
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, is not possible to report it as skipped instead of errord?

Copy link
Member Author

Choose a reason for hiding this comment

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

If so, then GitHub checks will be passed

image

and as consequence, there is a possibility the PR could be accidentally merged without running everything in the CI.

I'm inclined to be explicit with an artificial failure to avoid the above if it makes any sense.

@endorama
Copy link
Member

endorama commented Mar 1, 2022

cc @mtojek

@mtojek mtojek requested a review from a team March 1, 2022 14:18
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

Hold on folks, why is it necessary? What's the issue we're trying to solve?

@mtojek mtojek requested a review from jsoriano March 1, 2022 14:19
@v1v
Copy link
Member Author

v1v commented Mar 1, 2022

@mtojek , the why section in the description contains the reason, I kindly raised this based on the feedback from @endorama , you can freely close it if it's not needed :)

@mtojek
Copy link
Contributor

mtojek commented Mar 1, 2022

I see now, thanks for pointing it out, Victor. I believe that in the case of the Add GCP test package the problem is that we're waiting for the release v8.0.1, which should happen really soon. I wouldn't like to consider it as a justification to disable all tests, including successful ones.

If this is the only reason why we need this PR, I would be for not merging it.

@v1v v1v closed this Mar 2, 2022
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.

6 participants