Skip to content

Commit b5efa87

Browse files
ejoffeejoffe
committed
Add PR templates
Current template options include: - basic : just use the commit title and body - stack : like basic but also show stack of prs - custom : use custom pr template - why_what : first section of body explains why, second section is what commit-id:5d13e44a
1 parent 2f98ed9 commit b5efa87

File tree

17 files changed

+2320
-142
lines changed

17 files changed

+2320
-142
lines changed

‎config/config.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type RepoConfig struct {
3030
MergeMethod string `default:"rebase" yaml:"mergeMethod"`
3131
MergeQueue bool `default:"false" yaml:"mergeQueue"`
3232

33+
PRTemplateType string `default:"stack" yaml:"prTemplateType"`
3334
PRTemplatePath string `yaml:"prTemplatePath,omitempty"`
3435
PRTemplateInsertStart string `yaml:"prTemplateInsertStart,omitempty"`
3536
PRTemplateInsertEnd string `yaml:"prTemplateInsertEnd,omitempty"`
@@ -83,9 +84,21 @@ func DefaultConfig() *Config {
8384

8485
cfg.User.LogGitCommands = false
8586
cfg.User.LogGitHubCalls = false
87+
88+
// Normalize config (e.g., set PRTemplateType to "custom" if PRTemplatePath is provided)
89+
cfg.Normalize()
90+
8691
return cfg
8792
}
8893

94+
// Normalize applies normalization rules to the config
95+
// For example, if PRTemplatePath is provided, PRTemplateType should be set to "custom"
96+
func (c *Config) Normalize() {
97+
if c.Repo != nil && c.Repo.PRTemplatePath != "" {
98+
c.Repo.PRTemplateType = "custom"
99+
}
100+
}
101+
89102
func (c Config) MergeMethod() (genclient.PullRequestMergeMethod, error) {
90103
var mergeMethod genclient.PullRequestMergeMethod
91104
var err error

‎config/config_parser/config_parser.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ func ParseConfig(gitcmd git.GitInterface) *config.Config {
6161
rake.LoadSources(cfg.User,
6262
rake.YamlFileWriter(UserConfigFilePath()))
6363
}
64+
65+
// Normalize config (e.g., set PRTemplateType to "custom" if PRTemplatePath is provided)
66+
cfg.Normalize()
67+
6468
return cfg
6569
}
6670

‎config/config_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ func TestDefaultConfig(t *testing.T) {
3030
RequireChecks: true,
3131
RequireApproval: true,
3232
MergeMethod: "rebase",
33+
PRTemplateType: "stack",
3334
PRTemplatePath: "",
3435
PRTemplateInsertStart: "",
3536
PRTemplateInsertEnd: "",
@@ -90,3 +91,48 @@ func TestMergeMethodHelper(t *testing.T) {
9091
assert.Empty(t, actual)
9192
})
9293
}
94+
95+
func TestNormalizeConfig(t *testing.T) {
96+
t.Run("PRTemplatePath provided sets PRTemplateType to custom", func(t *testing.T) {
97+
cfg := &Config{
98+
Repo: &RepoConfig{
99+
PRTemplateType: "stack",
100+
PRTemplatePath: "/path/to/template.md",
101+
},
102+
}
103+
cfg.Normalize()
104+
assert.Equal(t, "custom", cfg.Repo.PRTemplateType)
105+
assert.Equal(t, "/path/to/template.md", cfg.Repo.PRTemplatePath)
106+
})
107+
108+
t.Run("PRTemplatePath empty does not change PRTemplateType", func(t *testing.T) {
109+
cfg := &Config{
110+
Repo: &RepoConfig{
111+
PRTemplateType: "stack",
112+
PRTemplatePath: "",
113+
},
114+
}
115+
cfg.Normalize()
116+
assert.Equal(t, "stack", cfg.Repo.PRTemplateType)
117+
assert.Equal(t, "", cfg.Repo.PRTemplatePath)
118+
})
119+
120+
t.Run("PRTemplatePath provided overrides existing PRTemplateType", func(t *testing.T) {
121+
cfg := &Config{
122+
Repo: &RepoConfig{
123+
PRTemplateType: "why_what",
124+
PRTemplatePath: "/custom/template.md",
125+
},
126+
}
127+
cfg.Normalize()
128+
assert.Equal(t, "custom", cfg.Repo.PRTemplateType)
129+
assert.Equal(t, "/custom/template.md", cfg.Repo.PRTemplatePath)
130+
})
131+
132+
t.Run("DefaultConfig with PRTemplatePath sets PRTemplateType to custom", func(t *testing.T) {
133+
cfg := DefaultConfig()
134+
cfg.Repo.PRTemplatePath = "/path/to/template.md"
135+
cfg.Normalize()
136+
assert.Equal(t, "custom", cfg.Repo.PRTemplateType)
137+
})
138+
}

‎github/githubclient/client.go‎

Lines changed: 26 additions & 137 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
package githubclient
22

33
import (
4-
"bytes"
54
"context"
65
"fmt"
76
"net/url"
87
"os"
98
"path"
10-
"path/filepath"
119
"strings"
1210

1311
"gopkg.in/yaml.v3"
@@ -17,6 +15,7 @@ import (
1715
"github.com/ejoffe/spr/github"
1816
"github.com/ejoffe/spr/github/githubclient/fezzik_types"
1917
"github.com/ejoffe/spr/github/githubclient/gen/genclient"
18+
"github.com/ejoffe/spr/github/template/config_fetcher"
2019
"github.com/rs/zerolog/log"
2120
"golang.org/x/oauth2"
2221
)
@@ -384,22 +383,14 @@ func (c *client) CreatePullRequest(ctx context.Context, gitcmd git.GitInterface,
384383
Str("FromBranch", headRefName).Str("ToBranch", baseRefName).
385384
Msg("CreatePullRequest")
386385

387-
body := formatBody(commit, info.PullRequests, c.config.Repo.ShowPrTitlesInStack)
388-
if c.config.Repo.PRTemplatePath != "" {
389-
pullRequestTemplate, err := readPRTemplate(gitcmd, c.config.Repo.PRTemplatePath)
390-
if err != nil {
391-
log.Fatal().Err(err).Msg("failed to read PR template")
392-
}
393-
body, err = insertBodyIntoPRTemplate(body, pullRequestTemplate, c.config.Repo, nil)
394-
if err != nil {
395-
log.Fatal().Err(err).Msg("failed to insert body into PR template")
396-
}
397-
}
386+
templatizer := config_fetcher.PRTemplatizer(c.config, gitcmd)
387+
388+
body := templatizer.Body(info, commit)
398389
resp, err := c.api.CreatePullRequest(ctx, genclient.CreatePullRequestInput{
399390
RepositoryId: info.RepositoryID,
400391
BaseRefName: baseRefName,
401392
HeadRefName: headRefName,
402-
Title: commit.Subject,
393+
Title: templatizer.Title(info, commit),
403394
Body: &body,
404395
Draft: &c.config.User.CreateDraftPRs,
405396
})
@@ -427,112 +418,6 @@ func (c *client) CreatePullRequest(ctx context.Context, gitcmd git.GitInterface,
427418
return pr
428419
}
429420

430-
func formatStackMarkdown(commit git.Commit, stack []*github.PullRequest, showPrTitlesInStack bool) string {
431-
var buf bytes.Buffer
432-
for i := len(stack) - 1; i >= 0; i-- {
433-
isCurrent := stack[i].Commit == commit
434-
var suffix string
435-
if isCurrent {
436-
suffix = " ⬅"
437-
} else {
438-
suffix = ""
439-
}
440-
var prTitle string
441-
if showPrTitlesInStack {
442-
prTitle = fmt.Sprintf("%s ", stack[i].Title)
443-
} else {
444-
prTitle = ""
445-
}
446-
447-
buf.WriteString(fmt.Sprintf("- %s#%d%s\n", prTitle, stack[i].Number, suffix))
448-
}
449-
450-
return buf.String()
451-
}
452-
453-
func formatBody(commit git.Commit, stack []*github.PullRequest, showPrTitlesInStack bool) string {
454-
if len(stack) <= 1 {
455-
return strings.TrimSpace(commit.Body)
456-
}
457-
458-
if commit.Body == "" {
459-
return fmt.Sprintf("**Stack**:\n%s",
460-
addManualMergeNotice(formatStackMarkdown(commit, stack, showPrTitlesInStack)))
461-
}
462-
463-
return fmt.Sprintf("%s\n\n---\n\n**Stack**:\n%s",
464-
commit.Body,
465-
addManualMergeNotice(formatStackMarkdown(commit, stack, showPrTitlesInStack)))
466-
}
467-
468-
// Reads the specified PR template file and returns it as a string
469-
func readPRTemplate(gitcmd git.GitInterface, templatePath string) (string, error) {
470-
repoRootDir := gitcmd.RootDir()
471-
fullTemplatePath := filepath.Clean(path.Join(repoRootDir, templatePath))
472-
pullRequestTemplateBytes, err := os.ReadFile(fullTemplatePath)
473-
if err != nil {
474-
return "", fmt.Errorf("%w: unable to read template %v", err, fullTemplatePath)
475-
}
476-
return string(pullRequestTemplateBytes), nil
477-
}
478-
479-
// insertBodyIntoPRTemplate inserts a text body into the given PR template and returns the result as a string.
480-
// It uses the PRTemplateInsertStart and PRTemplateInsertEnd values defined in RepoConfig to determine where the body
481-
// should be inserted in the PR template. If there are issues finding the correct place to insert the body
482-
// an error will be returned.
483-
//
484-
// NOTE: on PR update, rather than using the PR template, it will use the existing PR body, which should have
485-
// the PR template from the initial PR create.
486-
func insertBodyIntoPRTemplate(body, prTemplate string, repo *config.RepoConfig, pr *github.PullRequest) (string, error) {
487-
templateOrExistingPRBody := prTemplate
488-
if pr != nil && pr.Body != "" {
489-
templateOrExistingPRBody = pr.Body
490-
}
491-
492-
startPRTemplateSection, err := getSectionOfPRTemplate(templateOrExistingPRBody, repo.PRTemplateInsertStart, BeforeMatch)
493-
if err != nil {
494-
return "", fmt.Errorf("%w: PR template insert start = '%v'", err, repo.PRTemplateInsertStart)
495-
}
496-
497-
endPRTemplateSection, err := getSectionOfPRTemplate(templateOrExistingPRBody, repo.PRTemplateInsertEnd, AfterMatch)
498-
if err != nil {
499-
return "", fmt.Errorf("%w: PR template insert end = '%v'", err, repo.PRTemplateInsertEnd)
500-
}
501-
502-
return fmt.Sprintf("%v%v\n%v\n\n%v%v", startPRTemplateSection, repo.PRTemplateInsertStart, body,
503-
repo.PRTemplateInsertEnd, endPRTemplateSection), nil
504-
}
505-
506-
const (
507-
BeforeMatch = iota
508-
AfterMatch
509-
)
510-
511-
// getSectionOfPRTemplate searches text for a matching searchString and will return the text before or after the
512-
// match as a string. If there are no matches or more than one match is found, an error will be returned.
513-
func getSectionOfPRTemplate(text, searchString string, returnMatch int) (string, error) {
514-
split := strings.Split(text, searchString)
515-
switch len(split) {
516-
case 2:
517-
if returnMatch == BeforeMatch {
518-
return split[0], nil
519-
} else if returnMatch == AfterMatch {
520-
return split[1], nil
521-
}
522-
return "", fmt.Errorf("invalid enum value")
523-
case 1:
524-
return "", fmt.Errorf("no matches found")
525-
default:
526-
return "", fmt.Errorf("multiple matches found")
527-
}
528-
}
529-
530-
func addManualMergeNotice(body string) string {
531-
return body + "\n\n" +
532-
"⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). " +
533-
"Do not merge manually using the UI - doing so may have unexpected results.*"
534-
}
535-
536421
func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, pullRequests []*github.PullRequest, pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) {
537422

538423
if c.config.User.LogGitHubCalls {
@@ -548,33 +433,37 @@ func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
548433
Str("FromBranch", pr.FromBranch).Str("ToBranch", baseRefName).
549434
Interface("PR", pr).Msg("UpdatePullRequest")
550435

551-
body := formatBody(commit, pullRequests, c.config.Repo.ShowPrTitlesInStack)
552-
if c.config.Repo.PRTemplatePath != "" {
553-
pullRequestTemplate, err := readPRTemplate(gitcmd, c.config.Repo.PRTemplatePath)
554-
if err != nil {
555-
log.Fatal().Err(err).Msg("failed to read PR template")
556-
}
557-
body, err = insertBodyIntoPRTemplate(body, pullRequestTemplate, c.config.Repo, pr)
558-
if err != nil {
559-
log.Fatal().Err(err).Msg("failed to insert body into PR template")
436+
/*
437+
body := formatBody(commit, pullRequests, c.config.Repo.ShowPrTitlesInStack)
438+
if c.config.Repo.PRTemplatePath != "" {
439+
pullRequestTemplate, err := readPRTemplate(gitcmd, c.config.Repo.PRTemplatePath)
440+
if err != nil {
441+
log.Fatal().Err(err).Msg("failed to read PR template")
442+
}
443+
body, err = insertBodyIntoPRTemplate(body, pullRequestTemplate, c.config.Repo, pr)
444+
if err != nil {
445+
log.Fatal().Err(err).Msg("failed to insert body into PR template")
446+
}
560447
}
561-
}
562-
title := &commit.Subject
448+
title := &commit.Subject
449+
*/
563450

564451
input := genclient.UpdatePullRequestInput{
565452
PullRequestId: pr.ID,
566-
Title: title,
567-
Body: &body,
453+
// Title: title,
454+
// Body: &body,
568455
}
569456

570457
if !pr.InQueue {
571458
input.BaseRefName = &baseRefName
572459
}
573460

574-
if c.config.User.PreserveTitleAndBody {
575-
input.Title = nil
576-
input.Body = nil
577-
}
461+
/*
462+
if c.config.User.PreserveTitleAndBody {
463+
input.Title = nil
464+
input.Body = nil
465+
}
466+
*/
578467

579468
_, err := c.api.UpdatePullRequest(ctx, input)
580469

‎github/githubclient/client_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package githubclient
22

33
import (
4-
"strings"
54
"testing"
65

76
"github.com/ejoffe/spr/config"
@@ -530,6 +529,7 @@ func TestMatchPullRequestStack(t *testing.T) {
530529
}
531530
}
532531

532+
/*
533533
func TestFormatPullRequestBody(t *testing.T) {
534534
simpleCommit := git.Commit{
535535
CommitID: "abc123",
@@ -825,3 +825,4 @@ func TestInsertBodyIntoPRTemplateErrors(t *testing.T) {
825825
})
826826
}
827827
}
828+
*/

‎github/template/config_fetcher/config.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package config_fetcher
2+
3+
import (
4+
"github.com/ejoffe/spr/config"
5+
"github.com/ejoffe/spr/git"
6+
"github.com/ejoffe/spr/github/template"
7+
"github.com/ejoffe/spr/github/template/template_basic"
8+
"github.com/ejoffe/spr/github/template/template_custom"
9+
"github.com/ejoffe/spr/github/template/template_stack"
10+
"github.com/ejoffe/spr/github/template/template_why_what"
11+
)
12+
13+
func PRTemplatizer(c *config.Config, gitcmd git.GitInterface) template.PRTemplatizer {
14+
switch c.Repo.PRTemplateType {
15+
case "stack":
16+
return template_stack.NewStackTemplatizer(c.Repo.ShowPrTitlesInStack)
17+
case "basic":
18+
return template_basic.NewBasicTemplatizer()
19+
case "why_what":
20+
return template_why_what.NewWhyWhatTemplatizer()
21+
case "custom":
22+
return template_custom.NewCustomTemplatizer(c.Repo, gitcmd)
23+
default:
24+
return template_basic.NewBasicTemplatizer()
25+
}
26+
}

0 commit comments

Comments
 (0)