-
Notifications
You must be signed in to change notification settings - Fork 91
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
Misc cleanup of CI and README #2944
Conversation
rrsettgast
commented
Jan 18, 2024
•
edited
Loading
edited
- Cleanup badges in README.md
- fix ci_test.yml for non-pr runs
- add badge for actions status
…action is not triggered by a pull request
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2944 +/- ##
========================================
Coverage 51.33% 51.33%
========================================
Files 968 968
Lines 86819 86817 -2
========================================
Hits 44570 44570
+ Misses 42249 42247 -2 ☔ View full report in Codecov by Sentry. |
… and try to evaluate NUM_ASSIGNEES in if_not_unassigned_pull_request job instead of is_not_draft_pull_request
@@ -58,19 +56,22 @@ jobs: | |||
|
|||
# PR must be assigned to be merged. | |||
# This job will fail if this is not the case. | |||
is_pull_request_assigned: | |||
needs: [is_pull_request_a_draft] | |||
if_not_unassigned_pull_request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe names like unassigned_pr_guard
or draft_pr_guard
can be nice as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah...I struggled with a name...but this seemed pretty descriptive of the result. It is true if this action is "not on an unassigned pull request"....otherwise "it is an unassigned pull request" and it is a fail.
I think we should consider a more intrusive restructure of the yaml s.t. we have a "query" phase to get all the variables...then tests on those variables for things like (is_draft_pr, is_unassigned_pr, run_cuda_tests) then have the testing blocks depend on those instead of inferred....for instance the cuda tests only run if the label exists, but we have a test for a run_cuda label, then the tests depend on that. This way we can see the workflow better.
Co-authored-by: TotoGaz <49004943+TotoGaz@users.noreply.github.com>
* Cleanup badges in README.md * Added CI badge and docs badge * rename if_pull_request_is_assigned to if_not_unassigned_pull_request, and try to evaluate NUM_ASSIGNEES in if_not_unassigned_pull_request job instead of is_not_draft_pull_request * remove NUM_ASSIGNEES variable from yml --------- Co-authored-by: TotoGaz <49004943+TotoGaz@users.noreply.github.com>