chore(scripts): auto create autoversion PR from release script #13074
Conversation
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| @@ -0,0 +1,14 @@ | |||
| # Some documentation | |||
There was a problem hiding this comment.
Forgot to commit this test case in a previous PR.
scripts/release/main.go
Outdated
| @@ -385,7 +385,7 @@ func (r *releaseCommand) autoversionFile(ctx context.Context, file, channel, ver | |||
| // Utilize the index where the match was found to replace the correct part. The only | |||
| // match group is the version. | |||
| if match := matchRe.FindStringSubmatchIndex(line); match != nil { | |||
| logger.Info(ctx, "updating version number", "line_number", i+1, "match", match) | |||
| logger.Info(ctx, "updating version number", "line_number", i+1, "match", match, "old_version", line[match[2]:match[3]]) | |||
There was a problem hiding this comment.
Potential here for panic; can you check that match has the expected number of elements first?
There was a problem hiding this comment.
If the regex matches we can expect that match to contains all the elements, but I can add an extra check just to be safe.
There was a problem hiding this comment.
@dannykopping turned this into a more robust implementation in 26df0e5, wdyt?
scripts/release.sh
Outdated
| log "Creating pull request..." | ||
| maybedryrun "${dry_run}" gh pr create \ | ||
| --assignee "@me" \ | ||
| --reviewer bpmct,stirby \ |
There was a problem hiding this comment.
Can we define these in an arg as default values?
There was a problem hiding this comment.
Are you suggesting we define these at the top of the script? Or that we add CLI flags for them? Or perhaps neither of the aforementioned?
There was a problem hiding this comment.
Either one works, so long as the reviewers can be overridden if/when we need to without modifying the script
There was a problem hiding this comment.
I find it hard to imagine a scenario where being able to change this without modifying the script is useful 🤔. Even if it's there, it wouldn't be used and thus forgotten. The PR can always be edited on GitHub afterwards.
That's to say, I don't mind moving these vars to the top, and I can even add hidden envs for it, but not sure it's worth the effort to flag/document it.
04b8614 to
b29e24c
Compare
| @@ -53,6 +53,10 @@ script_check=1 | |||
| mainline=1 | |||
| channel=mainline | |||
|
|
|||
| # These values will be used for any PRs created. | |||
| pr_review_assignee=${CODER_RELEASE_PR_REVIEW_ASSIGNEE:-@me} | |||
| pr_review_reviewer=${CODER_RELEASE_PR_REVIEW_REVIEWER:-bpmct,stirby} | |||
There was a problem hiding this comment.
nit: maybe add yourself as reviewer too in case of emergency
There was a problem hiding this comment.
I don’t want to involve myself too much in the process, since this is just docs changes I don’t think I’m needed. I will cooperate with @stirby during the next release(s) to ensure the process is smooth.
Ref #12465