dashboard / erock/git-pr / Status comments #75 rss

accepted · opened on 2025-08-22T03:15:03Z by jolheiser
Help
checkout latest patchset:
ssh pr.pico.sh print pr-75 | git am -3
checkout any patchset in a patch request:
ssh pr.pico.sh print ps-X | git am -3
add changes to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add 75
add review to patch request:
git format-patch main --stdout | ssh pr.pico.sh pr add --review 75
accept PR:
ssh pr.pico.sh pr accept 75
close PR:
ssh pr.pico.sh pr close 75

Logs

jolheiser created pr with ps-147 on 2025-08-22T03:15:03Z
jolheiser changed pr name on 2025-08-22T03:15:55Z {"name":"Status comments"}
erock changed status on 2025-08-22T15:49:21Z {"status":"accepted","comment":"lgtm!"}

Patchsets

ps-147 by jolheiser on 2025-08-22T03:15:03Z

Patchset ps-147

strongly type status

jolheiser
2025-08-22T02:55:11Z
cli.go
+12 -12
models.go
+10 -1
pr.go
+2 -2
web.go
+6 -6

strongly type event data

jolheiser
2025-08-22T02:55:30Z
models.go
+38 -1
pr.go
+6 -2

add comments to status changes

jolheiser
2025-08-22T03:14:02Z
cli.go
+26 -4
models.go
+3 -2
pr.go
+4 -3
Back to top

strongly type status

Signed-off-by: jolheiser <git@jolheiser.com>
cli.go link
+12 -12
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
diff --git a/cli.go b/cli.go
index a4456c0..b1bd555 100644
--- a/cli.go
+++ b/cli.go
@@ -466,15 +466,15 @@ To get started, submit a new patch request:
 							writer := NewTabWriter(sesh)
 							fmt.Fprintln(writer, "ID\tRepoID\tName\tStatus\tPatchsets\tUser\tDate")
 							for _, req := range prs {
-								if onlyAccepted && req.Status != "accepted" {
+								if onlyAccepted && req.Status != StatusAccepted {
 									continue
 								}
 
-								if onlyClosed && req.Status != "closed" {
+								if onlyClosed && req.Status != StatusClosed {
 									continue
 								}
 
-								if onlyOpen && req.Status != "open" {
+								if onlyOpen && req.Status != StatusOpen {
 									continue
 								}
 
@@ -640,11 +640,11 @@ To get started, submit a new patch request:
 									return fmt.Errorf("you are not authorized to accept a PR")
 								}
 
-								if prq.Status == "accepted" {
+								if prq.Status == StatusAccepted {
 									return fmt.Errorf("PR has already been accepted")
 								}
 
-								err = pr.UpdatePatchRequestStatus(prID, user.ID, "accepted")
+								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusAccepted)
 								if err != nil {
 									return err
 								}
@@ -701,7 +701,7 @@ To get started, submit a new patch request:
 									return fmt.Errorf("you are not authorized to change PR status")
 								}
 
-								if prq.Status == "closed" {
+								if prq.Status == StatusClosed {
 									return fmt.Errorf("PR has already been closed")
 								}
 
@@ -710,7 +710,7 @@ To get started, submit a new patch request:
 									return err
 								}
 
-								err = pr.UpdatePatchRequestStatus(prID, user.ID, "closed")
+								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusClosed)
 								if err != nil {
 									return err
 								}
@@ -760,7 +760,7 @@ To get started, submit a new patch request:
 								return fmt.Errorf("you are not authorized to change PR status")
 							}
 
-							if prq.Status == "open" {
+							if prq.Status == StatusOpen {
 								return fmt.Errorf("PR is already open")
 							}
 
@@ -769,7 +769,7 @@ To get started, submit a new patch request:
 								return err
 							}
 
-							err = pr.UpdatePatchRequestStatus(prID, user.ID, "open")
+							err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusOpen)
 							if err == nil {
 								wish.Printf(sesh, "Reopened PR %s (#%d)\n", prq.Name, prq.ID)
 							}
@@ -887,17 +887,17 @@ To get started, submit a new patch request:
 							}
 
 							op := OpNormal
-							nextStatus := "open"
+							nextStatus := StatusOpen
 							if isReview {
 								wish.Println(sesh, "Marking patchset as a review")
 								op = OpReview
 							} else if isAccept {
 								wish.Println(sesh, "Marking PR as accepted")
-								nextStatus = "accepted"
+								nextStatus = StatusAccepted
 								op = OpAccept
 							} else if isClose {
 								wish.Println(sesh, "Marking PR as closed")
-								nextStatus = "closed"
+								nextStatus = StatusClosed
 								op = OpClose
 							}
 
models.go link
+10 -1
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
diff --git a/models.go b/models.go
index aecc90a..1218aac 100644
--- a/models.go
+++ b/models.go
@@ -7,6 +7,15 @@ import (
 	"github.com/bluekeyes/go-gitdiff/gitdiff"
 )
 
+type Status string
+
+const (
+	StatusOpen     Status = "open"
+	StatusClosed   Status = "closed"
+	StatusAccepted Status = "accepted"
+	StatusReviewed Status = "reviewed"
+)
+
 // User is a db model for users.
 type User struct {
 	ID        int64     `db:"id"`
@@ -41,7 +50,7 @@ type PatchRequest struct {
 	RepoID    int64     `db:"repo_id"`
 	Name      string    `db:"name"`
 	Text      string    `db:"text"`
-	Status    string    `db:"status"`
+	Status    Status    `db:"status"`
 	CreatedAt time.Time `db:"created_at"`
 	UpdatedAt time.Time `db:"updated_at"`
 	// only used for aggregate queries
pr.go link
+2 -2
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
diff --git a/pr.go b/pr.go
index 59664c7..b76a35c 100644
--- a/pr.go
+++ b/pr.go
@@ -43,7 +43,7 @@ type GitPatchRequest interface {
 	GetPatchsetByID(patchsetID int64) (*Patchset, error)
 	GetLatestPatchsetByPrID(prID int64) (*Patchset, error)
 	GetPatchesByPatchsetID(prID int64) ([]*Patch, error)
-	UpdatePatchRequestStatus(prID, userID int64, status string) error
+	UpdatePatchRequestStatus(prID, userID int64, status Status) error
 	UpdatePatchRequestName(prID, userID int64, name string) error
 	DeletePatchsetByID(userID, prID int64, patchsetID int64) error
 	CreateEventLog(tx *sqlx.Tx, eventLog EventLog) error
@@ -293,7 +293,7 @@ func (cmd PrCmd) GetPatchRequestByID(prID int64) (*PatchRequest, error) {
 }
 
 // Status types: open, closed, accepted, reviewed.
-func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status string) error {
+func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status Status) error {
 	tx, err := cmd.Backend.DB.Beginx()
 	if err != nil {
 		return err
web.go link
+6 -6
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
diff --git a/web.go b/web.go
index 468847c..fb58462 100644
--- a/web.go
+++ b/web.go
@@ -145,8 +145,8 @@ type RepoDetailData struct {
 func createPrDataSorter(sort, sortDir string) func(a, b *PrListData) int {
 	return func(a *PrListData, b *PrListData) int {
 		if sort == "status" {
-			statusA := strings.ToLower(a.Status)
-			statusB := strings.ToLower(b.Status)
+			statusA := strings.ToLower(string(a.Status))
+			statusB := strings.ToLower(string(b.Status))
 			if sortDir == "asc" {
 				return strings.Compare(statusA, statusB)
 			} else {
@@ -191,9 +191,9 @@ func createPrDataSorter(sort, sortDir string) func(a, b *PrListData) int {
 
 func getPrTableData(web *WebCtx, prs []*PatchRequest, query url.Values) ([]*PrListData, error) {
 	prdata := []*PrListData{}
-	status := strings.ToLower(query.Get("status"))
+	status := Status(strings.ToLower(query.Get("status")))
 	if status == "" {
-		status = "open"
+		status = StatusOpen
 	}
 	username := strings.ToLower(query.Get("user"))
 	title := strings.ToLower(query.Get("title"))
@@ -361,7 +361,7 @@ type PrListData struct {
 	ID           int64
 	DateOrig     time.Time
 	Date         string
-	Status       string
+	Status       Status
 }
 
 func userDetailHandler(w http.ResponseWriter, r *http.Request) {
@@ -518,7 +518,7 @@ type PrData struct {
 	ID     int64
 	Title  string
 	Date   string
-	Status string
+	Status Status
 }
 
 type PatchFile struct {

strongly type event data

Signed-off-by: jolheiser <git@jolheiser.com>
models.go link
+38 -1
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
diff --git a/models.go b/models.go
index 1218aac..8a01825 100644
--- a/models.go
+++ b/models.go
@@ -2,6 +2,9 @@ package git
 
 import (
 	"database/sql"
+	"database/sql/driver"
+	"encoding/json"
+	"fmt"
 	"time"
 
 	"github.com/bluekeyes/go-gitdiff/gitdiff"
@@ -97,6 +100,40 @@ type EventLog struct {
 	PatchRequestID sql.NullInt64 `db:"patch_request_id"`
 	PatchsetID     sql.NullInt64 `db:"patchset_id"`
 	Event          string        `db:"event"`
-	Data           string        `db:"data"`
 	CreatedAt      time.Time     `db:"created_at"`
+	Data           EventData     `db:"data"`
+}
+
+type EventData struct {
+	Name   string `json:"name,omitempty"`
+	Status Status `json:"status,omitempty"`
+}
+
+func (e EventData) String() string {
+	b, _ := json.Marshal(e)
+	bs := string(b)
+	if bs == "{}" {
+		return ""
+	}
+	return bs
+}
+
+func (e *EventData) Scan(value any) error {
+	if value == nil {
+		return nil
+	}
+	var bytes []byte
+	switch v := value.(type) {
+	case []byte:
+		bytes = v
+	case string:
+		bytes = []byte(v)
+	default:
+		return fmt.Errorf("cannot scan %T into EventData", value)
+	}
+	return json.Unmarshal(bytes, e)
+}
+
+func (e EventData) Value() (driver.Value, error) {
+	return json.Marshal(e)
 }
pr.go link
+6 -2
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
diff --git a/pr.go b/pr.go
index b76a35c..f3109fd 100644
--- a/pr.go
+++ b/pr.go
@@ -322,7 +322,9 @@ func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status Statu
 		RepoID:         sql.NullInt64{Int64: pr.RepoID, Valid: true},
 		PatchRequestID: sql.NullInt64{Int64: prID, Valid: true},
 		Event:          "pr_status_changed",
-		Data:           fmt.Sprintf(`{"status":"%s"}`, status),
+		Data: EventData{
+			Status: status,
+		},
 	})
 	if err != nil {
 		return err
@@ -364,7 +366,9 @@ func (cmd PrCmd) UpdatePatchRequestName(prID int64, userID int64, name string) e
 		RepoID:         sql.NullInt64{Int64: pr.RepoID, Valid: true},
 		PatchRequestID: sql.NullInt64{Int64: prID, Valid: true},
 		Event:          "pr_name_changed",
-		Data:           fmt.Sprintf(`{"name":"%s"}`, name),
+		Data: EventData{
+			Name: name,
+		},
 	})
 	if err != nil {
 		return err
tmpl/pages/pr.html link
+1 -1
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
diff --git a/tmpl/pages/pr.html b/tmpl/pages/pr.html
index 90532d3..35ddb14 100644
--- a/tmpl/pages/pr.html
+++ b/tmpl/pages/pr.html
@@ -42,7 +42,7 @@
           {{end}}
         </span>
         <span>on <date>{{.Date}}</date></span>
-        {{if .Data}}<code>{{.Data}}</code>{{end}}
+        {{if .Data.String}}<code>{{.Data}}</code>{{end}}
       </div>
       {{end}}
     </div>

add comments to status changes

Signed-off-by: jolheiser <git@jolheiser.com>
cli.go link
+26 -4
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
diff --git a/cli.go b/cli.go
index b1bd555..8f5e085 100644
--- a/cli.go
+++ b/cli.go
@@ -603,6 +603,12 @@ To get started, submit a new patch request:
 						Usage:     "Accept a PR",
 						Args:      true,
 						ArgsUsage: "[prID], [prID]...",
+						Flags: []cli.Flag{
+							&cli.StringFlag{
+								Name:  "comment",
+								Usage: "add a comment to the patchset(s)",
+							},
+						},
 						Action: func(cCtx *cli.Context) error {
 							args := cCtx.Args()
 							if !args.Present() {
@@ -644,7 +650,7 @@ To get started, submit a new patch request:
 									return fmt.Errorf("PR has already been accepted")
 								}
 
-								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusAccepted)
+								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusAccepted, cCtx.String("comment"))
 								if err != nil {
 									return err
 								}
@@ -664,6 +670,12 @@ To get started, submit a new patch request:
 						Usage:     "Close a PR",
 						Args:      true,
 						ArgsUsage: "[prID], [prID]...",
+						Flags: []cli.Flag{
+							&cli.StringFlag{
+								Name:  "comment",
+								Usage: "add a comment to the patchset(s)",
+							},
+						},
 						Action: func(cCtx *cli.Context) error {
 							args := cCtx.Args()
 							if !args.Present() {
@@ -710,7 +722,7 @@ To get started, submit a new patch request:
 									return err
 								}
 
-								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusClosed)
+								err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusClosed, cCtx.String("comment"))
 								if err != nil {
 									return err
 								}
@@ -729,6 +741,12 @@ To get started, submit a new patch request:
 						Usage:     "Reopen a PR",
 						Args:      true,
 						ArgsUsage: "[prID]",
+						Flags: []cli.Flag{
+							&cli.StringFlag{
+								Name:  "comment",
+								Usage: "add a comment to the patchset",
+							},
+						},
 						Action: func(cCtx *cli.Context) error {
 							args := cCtx.Args()
 							if !args.Present() {
@@ -769,7 +787,7 @@ To get started, submit a new patch request:
 								return err
 							}
 
-							err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusOpen)
+							err = pr.UpdatePatchRequestStatus(prID, user.ID, StatusOpen, cCtx.String("comment"))
 							if err == nil {
 								wish.Printf(sesh, "Reopened PR %s (#%d)\n", prq.Name, prq.ID)
 							}
@@ -847,6 +865,10 @@ To get started, submit a new patch request:
 								Name:  "close",
 								Usage: "submit patchset and mark PR as closed",
 							},
+							&cli.StringFlag{
+								Name:  "comment",
+								Usage: "add a comment to the patchset",
+							},
 						},
 						Action: func(cCtx *cli.Context) error {
 							args := cCtx.Args()
@@ -912,7 +934,7 @@ To get started, submit a new patch request:
 							}
 
 							if prq.Status != nextStatus {
-								err = pr.UpdatePatchRequestStatus(prID, user.ID, nextStatus)
+								err = pr.UpdatePatchRequestStatus(prID, user.ID, nextStatus, cCtx.String("comment"))
 								if err != nil {
 									return err
 								}
contrib/dev/main.go link
+5 -5
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
diff --git a/contrib/dev/main.go b/contrib/dev/main.go
index eba4a74..a4af8c8 100644
--- a/contrib/dev/main.go
+++ b/contrib/dev/main.go
@@ -68,17 +68,17 @@ func main() {
 	// Accepted patch
 	userKey.MustCmd(patch, "pr create test")
 	userKey.MustCmd(nil, "pr edit 1 Accepted patch")
-	adminKey.MustCmd(nil, "pr accept 1")
+	adminKey.MustCmd(nil, `pr accept --comment "lgtm!" 1`)
 
 	// Closed patch (admin)
 	userKey.MustCmd(patch, "pr create test")
 	userKey.MustCmd(nil, "pr edit 2 Closed patch (admin)")
-	adminKey.MustCmd(nil, "pr close 2")
+	adminKey.MustCmd(nil, `pr close --comment "Thanks for the effort! I think we might use PR #1 though." 2`)
 
 	// Closed patch (contributor)
 	userKey.MustCmd(patch, "pr create test")
 	userKey.MustCmd(nil, "pr edit 3 Closed patch (contributor)")
-	userKey.MustCmd(nil, "pr close 3")
+	userKey.MustCmd(nil, `pr close --comment "Woops, didn't mean to submit yet" 3`)
 
 	// Reviewed patch
 	userKey.MustCmd(patch, "pr create test")
@@ -88,12 +88,12 @@ func main() {
 	// Accepted patch with review
 	userKey.MustCmd(patch, "pr create test")
 	userKey.MustCmd(nil, "pr edit 5 Accepted patch with review")
-	adminKey.MustCmd(otherPatch, "pr add --accept 5")
+	adminKey.MustCmd(otherPatch, `pr add --accept --comment "L G T M" 5`)
 
 	// Closed patch with review
 	userKey.MustCmd(patch, "pr create test")
 	userKey.MustCmd(nil, "pr edit 6 Closed patch with review")
-	adminKey.MustCmd(otherPatch, "pr add --close 6")
+	adminKey.MustCmd(otherPatch, `pr add --close --comment "So close! I think we might try something else instead." 6`)
 
 	// Range Diff
 	userKey.MustCmd(rd1, "pr create test")
models.go link
+3 -2
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
diff --git a/models.go b/models.go
index 8a01825..26f4dec 100644
--- a/models.go
+++ b/models.go
@@ -105,8 +105,9 @@ type EventLog struct {
 }
 
 type EventData struct {
-	Name   string `json:"name,omitempty"`
-	Status Status `json:"status,omitempty"`
+	Name    string `json:"name,omitempty"`
+	Status  Status `json:"status,omitempty"`
+	Comment string `json:"comment,omitempty"`
 }
 
 func (e EventData) String() string {
pr.go link
+4 -3
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
diff --git a/pr.go b/pr.go
index f3109fd..049a223 100644
--- a/pr.go
+++ b/pr.go
@@ -43,7 +43,7 @@ type GitPatchRequest interface {
 	GetPatchsetByID(patchsetID int64) (*Patchset, error)
 	GetLatestPatchsetByPrID(prID int64) (*Patchset, error)
 	GetPatchesByPatchsetID(prID int64) ([]*Patch, error)
-	UpdatePatchRequestStatus(prID, userID int64, status Status) error
+	UpdatePatchRequestStatus(prID, userID int64, status Status, comment string) error
 	UpdatePatchRequestName(prID, userID int64, name string) error
 	DeletePatchsetByID(userID, prID int64, patchsetID int64) error
 	CreateEventLog(tx *sqlx.Tx, eventLog EventLog) error
@@ -293,7 +293,7 @@ func (cmd PrCmd) GetPatchRequestByID(prID int64) (*PatchRequest, error) {
 }
 
 // Status types: open, closed, accepted, reviewed.
-func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status Status) error {
+func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status Status, comment string) error {
 	tx, err := cmd.Backend.DB.Beginx()
 	if err != nil {
 		return err
@@ -323,7 +323,8 @@ func (cmd PrCmd) UpdatePatchRequestStatus(prID int64, userID int64, status Statu
 		PatchRequestID: sql.NullInt64{Int64: prID, Valid: true},
 		Event:          "pr_status_changed",
 		Data: EventData{
-			Status: status,
+			Status:  status,
+			Comment: comment,
 		},
 	})
 	if err != nil {