Skip to content

Commit f652f41

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 c76ad13 commit f652f41

File tree

20 files changed

+2654
-446
lines changed

20 files changed

+2654
-446
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: 16 additions & 140 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,113 +418,9 @@ 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-
536-
func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, pullRequests []*github.PullRequest, pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) {
421+
func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
422+
info *github.GitHubInfo, pullRequests []*github.PullRequest, pr *github.PullRequest,
423+
commit git.Commit, prevCommit *git.Commit) {
537424

538425
if c.config.User.LogGitHubCalls {
539426
fmt.Printf("> github update %d : %s\n", pr.Number, pr.Title)
@@ -548,34 +435,23 @@ func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
548435
Str("FromBranch", pr.FromBranch).Str("ToBranch", baseRefName).
549436
Interface("PR", pr).Msg("UpdatePullRequest")
550437

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")
560-
}
561-
}
562-
title := &commit.Subject
563-
438+
templatizer := config_fetcher.PRTemplatizer(c.config, gitcmd)
439+
title := templatizer.Title(info, commit)
440+
body := templatizer.Body(info, commit)
564441
input := genclient.UpdatePullRequestInput{
565442
PullRequestId: pr.ID,
566-
Title: title,
443+
Title: &title,
567444
Body: &body,
568445
}
569-
570-
if !pr.InQueue {
571-
input.BaseRefName = &baseRefName
572-
}
573-
574446
if c.config.User.PreserveTitleAndBody {
575447
input.Title = nil
576448
input.Body = nil
577449
}
578450

451+
if !pr.InQueue {
452+
input.BaseRefName = &baseRefName
453+
}
454+
579455
_, err := c.api.UpdatePullRequest(ctx, input)
580456

581457
if err != nil {

0 commit comments

Comments
 (0)