Skip to content
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

x/build/cmd/releasebot: "about to tag this commit, ok?" prompt UX can be improved #36948

Closed
dmitshur opened this issue Jan 31, 2020 · 1 comment
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jan 31, 2020

In the following method:

// gitTagVersion tags the release candidate or release in Git.
func (w *Work) gitTagVersion() {

It asks the user:

About to tag the following commit as goX.Y.Z:

[...]

Ok? (y/n)

I was used to pressing Shift+Y after using other apps like gcloud components update, which prompt:

Do you want to continue (Y/n)?

But when I did, two things happened:

  1. releasebot interpreted upper "Y" as a negative response.
  2. releasebot "stopped" by panicking with an error message that wasn't very clear or helpful.

Expand below for details:


[...]
2020/01/27 17:32:00 $ git rev-parse go1.13.7
About to tag the following commit as go1.13.7:

commit 7d2473dc81c659fba3f3b83bc6e93ca5fe37a898
Author: Dmitri Shuralyov <dmitshur@golang.org>
Date:   Mon Jan 27 16:36:12 2020 -0500

    [release-branch.go1.13-security] go1.13.7

    Change-Id: I4e9b0a8eee1ea6a0854eab88a2daf77b21da549a
    Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/649300
    Reviewed-by: Katie Hockman <katiehockman@google.com>

diff --git a/VERSION b/VERSION
index a92889e5b6..ab1e52b611 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-go1.13.6
\ No newline at end of file
+go1.13.7
\ No newline at end of file


Ok? (y/n) Y
2020/01/27 17:32:32 stopped
2020/01/27 17:32:32

PANIC: stopped

goroutine 1 [running]:
runtime/debug.Stack(0xc056a43960, 0x16033c0, 0xc058600030)
	/usr/local/go/src/runtime/debug/stack.go:24 +0x9d
main.(*Work).finally(0xc00fcdc0c0)
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:237 +0x65
panic(0x16033c0, 0xc058600030)
	/usr/local/go/src/runtime/panic.go:679 +0x1b2
log.(*Logger).Panic(0xc0528fb8b0, 0xc056a43b20, 0x1, 0x1)
	/usr/local/go/src/log/log.go:212 +0xaa
main.(*Work).gitTagVersion(0xc00fcdc0c0)
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/git.go:92 +0x646
main.(*Work).doRelease(0xc00fcdc0c0)
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:391 +0x5e0
main.main.func1(0xc03b80d3e0, 0x0, 0x0)
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:114 +0x17b
golang.org/x/build/maintner.(*GitHubRepo).ForeachMilestone(0xc002200000, 0xc056a43f18, 0x173a1ac, 0x2)
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/maintner/github.go:139 +0x9c
main.main()
	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:101 +0x4c0

## Latest build: go1\.13\.7

src not started
linux\-386 not started
linux\-armv6l not started
linux\-amd64 not started
linux\-arm64 not started
freebsd\-386 not started
freebsd\-amd64 not started
windows\-386 not started
windows\-amd64 not started
darwin\-amd64 not started
linux\-s390x not started
linux\-ppc64le not started

## Log

    2020/01/27 17:31:52 starting
    2020/01/27 17:31:52 working in /Users/dmitshur/go-releasebot-work/go1.13.7
    2020/01/27 17:31:52 $ git fetch origin master
    2020/01/27 17:31:53 $ git fetch origin release-branch.go1.13-security
    2020/01/27 17:31:55 $ git clone --reference /Users/dmitshur/go-releasebot-work/go1.13.7/gitmirror -b release-branch.go1.13-security sso://team/golang/go-private /Users/dmitshur/go-releasebot-work/go1.13.7/gitwork
    2020/01/27 17:31:59 $ git codereview change relwork
    2020/01/27 17:32:00 $ git config gc.auto 0
    2020/01/27 17:32:00 $ git rev-parse go1.13.7
    2020/01/27 17:32:32 stopped
    2020/01/27 17:32:32

    PANIC: stopped

    goroutine 1 [running]:
    runtime/debug.Stack(0xc056a43960, 0x16033c0, 0xc058600030)
    	/usr/local/go/src/runtime/debug/stack.go:24 +0x9d
    main.(*Work).finally(0xc00fcdc0c0)
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:237 +0x65
    panic(0x16033c0, 0xc058600030)
    	/usr/local/go/src/runtime/panic.go:679 +0x1b2
    log.(*Logger).Panic(0xc0528fb8b0, 0xc056a43b20, 0x1, 0x1)
    	/usr/local/go/src/log/log.go:212 +0xaa
    main.(*Work).gitTagVersion(0xc00fcdc0c0)
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/git.go:92 +0x646
    main.(*Work).doRelease(0xc00fcdc0c0)
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:391 +0x5e0
    main.main.func1(0xc03b80d3e0, 0x0, 0x0)
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:114 +0x17b
    golang.org/x/build/maintner.(*GitHubRepo).ForeachMilestone(0xc002200000, 0xc056a43f18, 0x173a1ac, 0x2)
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/maintner/github.go:139 +0x9c
    main.main()
    	/Users/dmitshur/go/pkg/mod/golang.org/x/build@v0.0.0-20200117212517-d3fb66653c7b/cmd/releasebot/main.go:101 +0x4c0


~ $

I suggest two changes to make things better for future releases:

  1. Make releasebot ask for and accept upper case "Y" in addition to lower case "y".

  2. Make the panic error message more descriptive than just "stopped", so it's faster to realize that releasebot is exiting because it received a negative answer. Additionally, exit with log.Fatalf rather than printing a panic stack trace.

For comparison, this is gcloud components update response to a negative answer:

[...]
Do you want to continue (Y/n)?  no

~ $ 
@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 31, 2020
@dmitshur dmitshur added this to the Unreleased milestone Jan 31, 2020
@dmitshur dmitshur changed the title x/build/cmd/releasebot: x/build/cmd/releasebot: "about to tag this commit, ok?" prompt UX can be improved Jan 31, 2020
@dmitshur dmitshur self-assigned this Sep 17, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 17, 2021
@dmitshur dmitshur added this to In Progress in Go Release Team Sep 17, 2021
@gopherbot
Copy link

Change https://golang.org/cl/350629 mentions this issue: cmd/releasebot: improve "about to tag this commit, ok?" prompt UX

Go Release Team automation moved this from In Progress to Done Sep 17, 2021
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
Archived in project
Development

No branches or pull requests

2 participants