run CI workflows for feat/ branches #1579
Conversation
|
+1 to the feat rule (we need to document this somewhere) -1 to not running CI on draft PRs, at least without someway to running them via a label or comment. I like to use draft PRs to mean "this isn't ready yet for review" not "don't run CI on this". Sometimes I am not ready for review but I do want to see if it passes CI. But there are times when I don't want to run CI on a PR so I think in a perfect world if I could have a label that I can add to a draft PR that kicks off CI, that would be grand (and the label is a no op when the PR isn't in draft mode) |
|
I agree with Mike, running on draft PRs often helps to inform when I should request a review. If things are looking green, that's a good sign that I am actually ready to have this seriously looked at. |
feat/ rule documented in this draft of contributing guidelines: #1580.
This is a good point, are you saying "don't run tests on drafts by default, but add a comment or label that overrides it and turns on testing"? |
IAlibay
left a comment
IAlibay
left a comment
There was a problem hiding this comment.
Feels like there was a discussion somewhere about best prapctices that I didn't look at. I don't want to block, but at the same time I have a lot of questions.
.github/workflows/ci.yaml
Outdated
| paths-ignore: | ||
| - "docs/*" | ||
| - ".readthedocs.yaml" | ||
| types: [ready_for_review] # don't run for draft PRs |
There was a problem hiding this comment.
See my comment in #1580, also in my opinion this discourages the use of draft PRs. If I open a draft PR, I also want to run CI to see what happens.
If we're concerned about running CI too much on draft, then we should just reduce the number of CI runners we run (we don't need to run codecov for every flavour of Python, and we have way too many CI runner types).
.github/workflows/ci.yaml
Outdated
| pull_request: | ||
| branches: | ||
| - main | ||
| - 'feat/**' |
There was a problem hiding this comment.
Why just feat? I often write fixes going into other PRs that would need CI to run on them.
Yes, but only if there is some push for not wanting to run CI on drafts b/c it is an obvious waste and most of the time, the author of the PR didn't care about CI while in draft mode. |
atravitz
changed the title
atravitz
force-pushed
the
ci/update_rules
branch
from
a3d82a7 to
c6ac173
Compare
|
No API break detected ✅ |
|
addressed by github.com//issues/1649 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1579 +/- ##
==========================================
- Coverage 95.41% 93.01% -2.41%
==========================================
Files 185 185
Lines 16045 16045
==========================================
- Hits 15310 14924 -386
- Misses 735 1121 +386
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
In conversation with @ethanholz, we discussed running CI for branches that start with
feat/, so that we can use CI for feature branches that might have many smaller PRs going into them.Also, I'm proposing that we don't auto-run CI for PRs in draft-mode to save the rainforest.
Checklist
newsentryDevelopers certificate of origin