Support "all failed jobs" and individual job re-run for a github actions workflow run#5275
Support "all failed jobs" and individual job re-run for a github actions workflow run#5275
Conversation
vilmibm
left a comment
There was a problem hiding this comment.
thanks for this!
i tested the argument processing stuff and it seems good 👍 I would like to QA this again once the feature flag is flipped and before we release on Tuesday.
|
@vilmibm - if you use it against github staff repos, you can QA it now as it is staff shipped, or I can flag in any additional repos, just message me |
|
oh sick--can you add vilmibm/testing? |
|
@vilmibm Done! |
mislav
left a comment
There was a problem hiding this comment.
This looks good! Housekeeping items remain
pkg/cmd/run/rerun/rerun.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| if opts.IO.CanPrompt() { |
There was a problem hiding this comment.
I realize it was like this before the feature additions, but if we could make just a bit of housekeeping while we're here, I would suggest replacing these CanPrompt() guards with IsStdoutTTY(). The reason is that we're not checking for whether the command can prompt the user, but whether the output is going to a terminal and not, for example, as part of a script. These checks are often aligned but not always.
pkg/cmd/run/rerun/rerun.go
Outdated
| opts.RunID = "" | ||
| if opts.IO.CanPrompt() { | ||
| cs := opts.IO.ColorScheme() | ||
| fmt.Fprintf(opts.IO.ErrOut, "%s both run and job IDs specified; ignoring run ID\n", cs.WarningIcon()) |
There was a problem hiding this comment.
Instead of ignoring a piece of input, should we error out in this case?
There was a problem hiding this comment.
Sure! I'll return cmdutil.FlagErrorf instead.
I took the pattern from
Lines 130 to 135 in d4ead71
There was a problem hiding this comment.
For the sake of backwards compatibility, let's not change where it was allowed before
pkg/cmd/run/rerun/rerun.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| if opts.IO.CanPrompt() { |
There was a problem hiding this comment.
Ditto CanPrompt → IsStdoutTTY
pkg/cmd/run/rerun/rerun.go
Outdated
| cs := opts.IO.ColorScheme() | ||
| fmt.Fprintf(opts.IO.ErrOut, "%s both run and job IDs specified; ignoring run ID\n", cs.WarningIcon()) | ||
| } | ||
| return cmdutil.FlagErrorf("specify only one of <run-id> or <job-id>") |
There was a problem hiding this comment.
Nit: since <job-id> is not a placeholder anywhere, it can be confusing to see this. When we're referring to flags, we usually name them by their full usage: --job (in backticks)
This PR implements 2 new flags to the
gh run rerunworkflow, to support the upcoming work to allow running only the failed jobs from a GitHub Actions workflow run (--failed) and a specific job from a workflow (--job <job-id>).The endpoints referenced here are NOT yet released publicly, so this PR should not be merged yet, but hopefully can merge shortly after we launch.
Our internal issue tracking this is https://github.com/github/cli/issues/110, let me know if I should create one in this repo as well.
Also first time contributing here so open to any and all feedback, thanks in advance!! 🙇