Monorepo for Tangled tangled.org

hooks: add create PR message on git push #612

Respond to a successful push with a URL to create a PR pointing to the default branch. This behavior is made to mimic other Git forges.

Signed-off-by: Samuel Shuert me@thecoded.prof

Labels
enhancement
assignee

None yet.

Participants 2
AT URI
at://did:plc:k2zmz2l3hvfr44tmlhewol2j/sh.tangled.repo.pull/3lzwhi232k322
+45
Diff #2
+45
knotserver/internal.go
··· 15 15 "github.com/go-chi/chi/v5/middleware" 16 16 "tangled.org/core/api/tangled" 17 17 "tangled.org/core/hook" 18 + "tangled.org/core/idresolver" 18 19 "tangled.org/core/knotserver/config" 19 20 "tangled.org/core/knotserver/db" 20 21 "tangled.org/core/knotserver/git" ··· 118 119 // non-fatal 119 120 } 120 121 122 + if (line.NewSha.String() != line.OldSha.String()) && line.OldSha.IsZero() { 123 + msg, err := h.replyCompare(line, gitUserDid, gitRelativeDir, repoName, r.Context()) 124 + if err != nil { 125 + l.Error("failed to reply with compare link", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir) 126 + // non-fatal 127 + } else { 128 + for msgLine := range msg { 129 + resp.Messages = append(resp.Messages, msg[msgLine]) 130 + } 131 + } 132 + } 133 + 121 134 err = h.triggerPipeline(&resp.Messages, line, gitUserDid, repoDid, repoName, pushOptions) 122 135 if err != nil { 123 136 l.Error("failed to trigger pipeline", "err", err, "line", line, "did", gitUserDid, "repo", gitRelativeDir) ··· 128 141 writeJSON(w, resp) 129 142 } 130 143 144 + func (h *InternalHandle) replyCompare(line git.PostReceiveLine, gitUserDid string, gitRelativeDir string, repoName string, ctx context.Context) ([]string, error) { 145 + l := h.l.With("handler", "replyCompare") 146 + userIdent, err := idresolver.DefaultResolver().ResolveIdent(ctx, gitUserDid) 147 + user := gitUserDid 148 + if err != nil { 149 + l.Error("Failed to fetch user identity", "err", err) 150 + // non-fatal 151 + } else { 152 + user = userIdent.Handle.String() 153 + } 154 + gr, err := git.PlainOpen(gitRelativeDir) 155 + if err != nil { 156 + l.Error("Failed to open git repository", "err", err) 157 + return []string{}, err 158 + } 159 + defaultBranch, err := gr.FindMainBranch() 160 + if err != nil { 161 + l.Error("Failed to fetch default branch", "err", err) 162 + return []string{}, err 163 + } 164 + if line.Ref == fmt.Sprintf("refs/heads/%s", defaultBranch) { 165 + return []string{}, nil 166 + } 167 + ZWS := "โ€‹" 168 + var msg []string 169 + msg = append(msg, ZWS) 170 + msg = append(msg, fmt.Sprintf("Create a PR pointing to %s", defaultBranch)) 171 + msg = append(msg, fmt.Sprintf("\t%s/%s/%s/compare/%s...%s", h.c.AppViewEndpoint, user, repoName, defaultBranch, strings.TrimPrefix(line.Ref, "refs/heads/"))) 172 + msg = append(msg, ZWS) 173 + return msg, nil 174 + } 175 + 131 176 func (h *InternalHandle) insertRefUpdate(line git.PostReceiveLine, gitUserDid, repoDid, repoName string) error { 132 177 didSlashRepo, err := securejoin.SecureJoin(repoDid, repoName) 133 178 if err != nil {

History

5 rounds 3 comments
sign up or login to add to the discussion
1 commit
expand
knotserver/internal: add create PR message on git push
expand 1 comment

lgtm thanks!

pull request successfully merged
1 commit
expand
knotserver/internal: add create PR message on git push
expand 0 comments
1 commit
expand
knotserver/internal: add create PR message on git push
expand 0 comments
1 commit
expand
knotserver: add create PR message on git push
expand 1 comment

this looks pretty good now! the only thing i'd want from this is for this handler to be separated out, just like how insertRefUpdate and triggerPipeline are split up.

this is because we don't want errors here to end the function, even the userIdent fails to resolve, we want to continue with the rest of the post-receive handler.

1 commit
expand
hooks: add create PR message on git push
expand 1 comment

i would prefer if this addition were made to knotserver/internal.go along side the other post-recieve handlers (insertRefUpdate and triggerPipeline). the way to return strings to the user in this handler are via the HookResponse struct.

there is an example of this in triggerPipeline which conditionally returns pipeline compilation status back to the user, by writing messages into resp.Messages. we could do something similar here.

that being said, the change largely looks good to me! thanks for working on this.