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: disable runs on draft prs #1380

Merged
merged 23 commits into from
Oct 9, 2023
Merged

ci: disable runs on draft prs #1380

merged 23 commits into from
Oct 9, 2023

Conversation

strophy
Copy link
Collaborator

@strophy strophy commented Sep 8, 2023

Issue being fixed or feature implemented

To reduce costs, we should only run CI when devs are ready for a check.

What was done?

Check for ci label on PRs before triggering self-hosted CI jobs

How Has This Been Tested?

In this PR by adding/removing the tag and doing test pushes

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@strophy strophy added this to the v0.25.0 milestone Sep 8, 2023
@strophy
Copy link
Collaborator Author

strophy commented Sep 8, 2023

This PR only disables self-hosted CI, Github-hosted actions will still run. Should we consider reducing the frequency of the scheduled run as well?

@strophy strophy marked this pull request as ready for review September 8, 2023 02:19
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

I believe we want runs for drafts

@strophy strophy added the ci label Sep 11, 2023
@strophy strophy removed the ci label Sep 11, 2023
@strophy strophy added the ci label Sep 11, 2023
@strophy strophy added ci and removed ci labels Sep 11, 2023
@strophy
Copy link
Collaborator Author

strophy commented Sep 11, 2023

@QuantumExplorer please review, we changed the task slightly to set a label instead of draft status, so that we can still run CI on drafts, and use the draft status to indicate whether something is ready to be merged yet or not

Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

CI won't work even if pull request is not a draft but doesn't have ci label?

@strophy
Copy link
Collaborator Author

strophy commented Sep 11, 2023

Right, the ci label is now required to run CI

@shumkov
Copy link
Member

shumkov commented Sep 12, 2023

So we adding an extra step to add label? What if you didn't add this label? PR looks green even without running workflows? TBH, this improvement looks to me as a complication of our development process and adds risks to merge PRs without testing. @lklimek and @markin-io your thoughts?

@strophy
Copy link
Collaborator Author

strophy commented Sep 13, 2023

@QuantumExplorer you requested this, can you guys please align on what you actually want and comment here?

@strophy strophy changed the title ci: disable runs on draft prs ci: disable CI runs until ci label is added to PR Sep 18, 2023
@QuantumExplorer
Copy link
Member

My request that we no longer run CI for draft PRs. I push many many times a day when I'm in active development. @shumkov why would you want CI on drafts?

@shumkov
Copy link
Member

shumkov commented Sep 21, 2023

To see all tests are passing before I request a review

@shumkov
Copy link
Member

shumkov commented Sep 22, 2023

@strophy Talked to @QuantumExplorer. Please just do not run CI for drafts. No label logic. Trigger CI when we switch from draft to normal PR. So in my case I will just switch to normal PR wait for CI and then switch back to draft.

@strophy strophy marked this pull request as draft October 3, 2023 08:07
@strophy strophy removed the ci label Oct 3, 2023
@strophy strophy marked this pull request as ready for review October 3, 2023 08:10
@strophy strophy requested a review from shumkov October 9, 2023 06:10
@strophy strophy changed the title ci: disable CI runs until ci label is added to PR ci: disable runs on draft prs Oct 9, 2023
Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

👍

@strophy strophy merged commit 6bdade8 into v0.25-dev Oct 9, 2023
50 checks passed
@strophy strophy deleted the ci/disable-draft-runs branch October 9, 2023 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants