mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-12 23:54:01 +09:00
fix(release): print the batch pull request's URL when --push publishes it
run --push opened or updated the batch pull request and said nothing about which one, so the scheduled job's log and a maintainer's terminal had no link to follow. gh pr create prints the URL; the run now passes it on, on stderr, so stdout stays the body alone, which the scheduled script saves as it is. An update prints the open pull request's URL. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
This commit is contained in:
6 files changed
+57
-14
No files matched your search
@@ -36,7 +36,7 @@ The pull request body goes to stdout, built from the line's own `.github/PULL_RE
|
||||
|
||||
`--defer N[,N] --reason TEXT` takes originals out of the open batch pull request, usually the ones a failing check was pinned on. It builds the batch branch again off `origin/<line>` without their units: while the batch still sits on the line's head, the units before the first one it drops stay as they are, and every later one, a pushed resolution included, is picked again in its own shape, a merge's picks one by one and then the merge with its message. When dropping one leaves a later commit that no longer applies, the run stops and names that commit's original, to defer as well. It adds no new original. It also takes out an original that got `backport/skip` after the batch picked it, which a run never removes on its own and the body lists as still on the branch. The body records each deferred original with the reason in the `backport-batch` block; a run that continues this batch keeps them out, and the next batch tries them again. A batch that carries a merge commit other than an original's, as GitHub's update-branch button makes, is refused, since picking such a merge again would lose what it resolved.
|
||||
|
||||
`--push` pushes the branch, refusing anything but a fast-forward of what the remote has, then opens the draft pull request against the line with `gh pr create --draft`, or updates the open batch pull request with `gh pr edit`. When the branch it refuses to push over has no pull request open on it, as a run that stopped between its push and opening the pull request leaves it, the refusal names the `run --onto <its head> --push` that continues it. While a batch pull request is open on another branch it refuses, so a line never has two. It never pushes to or edits a batch pull request whose body has no `backport-batch` block either: someone wrote that body by hand, and the batch is theirs to merge or close. After `--defer` alone it replaces the branch, with `--force-with-lease=refs/heads/<branch>:<head>`, where head is what `git ls-remote` reads for the branch and has to be the head the rebuild started from: a commit pushed since is never dropped, by the tool's check or, if one lands between that check and the push, by the remote.
|
||||
`--push` pushes the branch, refusing anything but a fast-forward of what the remote has, then opens the draft pull request against the line with `gh pr create --draft`, or updates the open batch pull request with `gh pr edit`, and prints its URL on stderr. When the branch it refuses to push over has no pull request open on it, as a run that stopped between its push and opening the pull request leaves it, the refusal names the `run --onto <its head> --push` that continues it. While a batch pull request is open on another branch it refuses, so a line never has two. It never pushes to or edits a batch pull request whose body has no `backport-batch` block either: someone wrote that body by hand, and the batch is theirs to merge or close. After `--defer` alone it replaces the branch, with `--force-with-lease=refs/heads/<branch>:<head>`, where head is what `git ls-remote` reads for the branch and has to be the head the rebuild started from: a commit pushed since is never dropped, by the tool's check or, if one lands between that check and the push, by the remote.
|
||||
|
||||
### check
|
||||
|
||||
|
||||
@@ -517,9 +517,26 @@ func (a *app) publish(b *batch, branch, head, title, body string, rebuilt bool)
|
||||
return err
|
||||
}
|
||||
if b.pr == nil {
|
||||
return a.gh.createPull(b.line.branch(), branch, title, f.Name())
|
||||
url, err := a.gh.createPull(b.line.branch(), branch, title, f.Name())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
fmt.Fprintf(a.stderr, "opened draft %s\n", url)
|
||||
return nil
|
||||
}
|
||||
return a.gh.editPull(b.pr.Number, title, f.Name())
|
||||
if err := a.gh.editPull(b.pr.Number, title, f.Name()); err != nil {
|
||||
return err
|
||||
}
|
||||
fmt.Fprintf(a.stderr, "updated %s\n", pullURL(b.pr))
|
||||
return nil
|
||||
}
|
||||
|
||||
// pullURL is where a pull request is on GitHub.
|
||||
func pullURL(pr *pullRequest) string {
|
||||
if pullURLRE.MatchString(pr.HTMLURL) {
|
||||
return pr.HTMLURL
|
||||
}
|
||||
return fmt.Sprintf("%spull/%d", ourRepoURL, pr.Number)
|
||||
}
|
||||
|
||||
// pushArgs pushes head to branch: as a fast-forward, which the remote checks,
|
||||
|
||||
@@ -258,9 +258,17 @@ func TestRunPush(t *testing.T) {
|
||||
f.lines()
|
||||
f.pr(5, "fix(app): five", "", map[string]string{"app/five.txt": "5\n"})
|
||||
gh := &fakeGH{}
|
||||
if code, _, stderr := f.exec(gh, "run", "--line", "release-1.1", "--push"); code != 0 {
|
||||
code, stdout, stderr := f.exec(gh, "run", "--line", "release-1.1", "--push")
|
||||
if code != 0 {
|
||||
t.Fatalf("run --push = %d\n%s", code, stderr)
|
||||
}
|
||||
if !strings.Contains(stderr, "opened draft https://github.com/cozystack/cozystack/pull/90\n") {
|
||||
t.Errorf("the run does not print the pull request it opened:\n%s", stderr)
|
||||
}
|
||||
l, _ := parseLine("release-1.1")
|
||||
if _, err := parseDeferred(stdout, l); err != nil || !strings.HasPrefix(stdout, "## What this PR does") || strings.Contains(stdout, "https://github.com/cozystack/cozystack/pull/90") {
|
||||
t.Errorf("stdout is no longer the body alone (%v):\n%s", err, stdout)
|
||||
}
|
||||
head := f.git("rev-parse", "HEAD")
|
||||
if got := f.remoteHead(batchBranch1); got != head {
|
||||
t.Errorf("remote %s = %q, want %s", batchBranch1, got, head)
|
||||
@@ -272,9 +280,13 @@ func TestRunPush(t *testing.T) {
|
||||
f.git("switch", "-q", "main")
|
||||
f.pr(6, "fix(app): six", "", map[string]string{"app/six.txt": "6\n"})
|
||||
gh.prs = []pullRequest{f.batchPR(90, "release-1.1", batchBranch1, gh.bodies[0])}
|
||||
if code, _, stderr := f.exec(gh, "run", "--line", "release-1.1", "--onto", head, "--push"); code != 0 {
|
||||
code, _, stderr = f.exec(gh, "run", "--line", "release-1.1", "--onto", head, "--push")
|
||||
if code != 0 {
|
||||
t.Fatalf("run --onto --push = %d\n%s", code, stderr)
|
||||
}
|
||||
if !strings.Contains(stderr, "updated https://github.com/cozystack/cozystack/pull/90\n") {
|
||||
t.Errorf("the run does not print the pull request it updated:\n%s", stderr)
|
||||
}
|
||||
if got := f.subjects("origin/release-1.1", f.remoteHead(batchBranch1)); !slices.Equal(got, []string{"fix(app): five (#5)", "fix(app): six (#6)"}) {
|
||||
t.Errorf("the pushed batch carries %q, want #5 then #6", got)
|
||||
}
|
||||
|
||||
@@ -309,9 +309,9 @@ func (g *fakeGH) openPulls(base string) ([]pullRequest, error) {
|
||||
return out, g.err
|
||||
}
|
||||
|
||||
func (g *fakeGH) createPull(base, head, title, bodyFile string) error {
|
||||
func (g *fakeGH) createPull(base, head, title, bodyFile string) (string, error) {
|
||||
g.created = append(g.created, base+" "+head+" "+title)
|
||||
return g.readBody(bodyFile)
|
||||
return fmt.Sprintf("https://github.com/%s/pull/90", upstreamRepo), g.readBody(bodyFile)
|
||||
}
|
||||
|
||||
func (g *fakeGH) editPull(number int, title, bodyFile string) error {
|
||||
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// openPullsCap is the page size of the open pull request listing. The REST
|
||||
@@ -43,6 +44,7 @@ var (
|
||||
batchBranchRE = regexp.MustCompile(`^backport-batch/(release-\d+\.\d+)-(\d{8})$`)
|
||||
loginRE = regexp.MustCompile(`^[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?$`)
|
||||
botLoginRE = regexp.MustCompile(`^[A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?\[bot\]$`)
|
||||
pullURLRE = regexp.MustCompile(`^https://[^/\s]+/[^/\s]+/[^/\s]+/pull/[1-9][0-9]*$`)
|
||||
)
|
||||
|
||||
// batchBotEnv names the GitHub App bot the scheduled batch opens its pull
|
||||
@@ -107,7 +109,8 @@ type github interface {
|
||||
permission(login string) (string, error)
|
||||
// labelled lists the pull requests merged into main that carry label.
|
||||
labelled(label string) ([]int, error)
|
||||
createPull(base, head, title, bodyFile string) error
|
||||
// createPull opens the draft and returns its URL.
|
||||
createPull(base, head, title, bodyFile string) (string, error)
|
||||
editPull(number int, title, bodyFile string) error
|
||||
comments(number int) ([]issueComment, error)
|
||||
createComment(number int, bodyFile string) error
|
||||
@@ -175,9 +178,16 @@ func (ghCLI) labelled(label string) ([]int, error) {
|
||||
return numbers, nil
|
||||
}
|
||||
|
||||
func (ghCLI) createPull(base, head, title, bodyFile string) error {
|
||||
_, err := capture("gh", "pr", "create", "--draft", "--base", base, "--head", head, "--title", title, "--body-file", bodyFile)
|
||||
return err
|
||||
func (ghCLI) createPull(base, head, title, bodyFile string) (string, error) {
|
||||
out, err := capture("gh", "pr", "create", "--draft", "--base", base, "--head", head, "--title", title, "--body-file", bodyFile)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
url := strings.TrimSpace(out)
|
||||
if !pullURLRE.MatchString(url) {
|
||||
return "", fmt.Errorf("gh pr create printed %q, not the URL of a pull request", url)
|
||||
}
|
||||
return url, nil
|
||||
}
|
||||
|
||||
func (ghCLI) editPull(number int, title, bodyFile string) error {
|
||||
|
||||
@@ -230,13 +230,17 @@ func TestGHCLI(t *testing.T) {
|
||||
|
||||
// A batch pull request carries no label: the release workflows act on a
|
||||
// pull request cozystack-ci[bot] opened only when it has the release label.
|
||||
write("")
|
||||
if err := (ghCLI{}).createPull("release-1.7", batchBranch1, "fix(backport): batch", "/tmp/body.md"); err != nil {
|
||||
t.Fatal(err)
|
||||
write("https://github.com/cozystack/cozystack/pull/5300\n")
|
||||
if url, err := (ghCLI{}).createPull("release-1.7", batchBranch1, "fix(backport): batch", "/tmp/body.md"); err != nil || url != "https://github.com/cozystack/cozystack/pull/5300" {
|
||||
t.Errorf("createPull = %q, %v; want the URL gh printed", url, err)
|
||||
}
|
||||
if want := "pr create --draft --base release-1.7 --head " + batchBranch1 + " --title fix(backport): batch --body-file /tmp/body.md"; called() != want {
|
||||
t.Errorf("gh %q, want gh %q", called(), want)
|
||||
}
|
||||
write("Warning: 1 uncommitted change\n")
|
||||
if _, err := (ghCLI{}).createPull("release-1.7", batchBranch1, "fix(backport): batch", "/tmp/body.md"); err == nil {
|
||||
t.Error("createPull took output that is no pull request URL")
|
||||
}
|
||||
write("")
|
||||
if err := (ghCLI{}).editPull(90, "fix(backport): batch", "/tmp/body.md"); err != nil {
|
||||
t.Fatal(err)
|
||||
|
||||
Reference in new issue
Block a user