Skip to content

Commit 9d782cf

Browse files
JoeWang1127JoeWang1127
authored
feat: add pr body to generate pull request (#1909)
Fixes #1703
1 parent 70ce073 commit 9d782cf

File tree

10 files changed

+339
-343
lines changed

10 files changed

+339
-343
lines changed

‎internal/github/github.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,9 @@ func (c *Client) CreatePullRequest(ctx context.Context, repo *Repository, remote
143143
slog.Warn("Provided PR body is empty, setting default.")
144144
body = "Regenerated all changed APIs. See individual commits for details."
145145
}
146-
slog.Info("Creating PR", "branch", remoteBranch, "title", title, "body", body)
146+
slog.Info("Creating PR", "branch", remoteBranch, "base", baseBranch, "title", title)
147+
// The body may be excessively long, only display in debug mode.
148+
slog.Debug("with PR body", "body", body)
147149
newPR := &github.NewPullRequest{
148150
Title: &title,
149151
Head: &remoteBranch,

‎internal/gitrepo/gitrepo.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ func (r *LocalRepository) AddAll() (git.Status, error) {
174174
// Commit creates a new commit with the provided message and author
175175
// information.
176176
func (r *LocalRepository) Commit(msg string) error {
177+
slog.Info("Committing", "message", msg)
177178
worktree, err := r.repo.Worktree()
178179
if err != nil {
179180
return err
@@ -424,7 +425,7 @@ func (r *LocalRepository) CreateBranchAndCheckout(name string) error {
424425
func (r *LocalRepository) Push(branchName string) error {
425426
// https://stackoverflow.com/a/75727620
426427
refSpec := config.RefSpec(fmt.Sprintf("+refs/heads/%s:refs/heads/%s", branchName, branchName))
427-
slog.Info("Pushing changes", slog.Any("refspec", refSpec))
428+
slog.Info("Pushing changes", "branch name", branchName, slog.Any("refspec", refSpec))
428429
var auth *httpAuth.BasicAuth
429430
if r.gitPassword != "" {
430431
slog.Info("Authenticating with basic auth")

‎internal/librarian/command.go‎

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@ import (
2929
"strings"
3030
"time"
3131

32-
"github.com/googleapis/librarian/internal/conventionalcommits"
33-
3432
"github.com/googleapis/librarian/internal/docker"
3533

3634
"github.com/googleapis/librarian/internal/config"
@@ -44,13 +42,14 @@ const (
4442
)
4543

4644
type commitInfo struct {
47-
cfg *config.Config
48-
state *config.LibrarianState
49-
repo gitrepo.Repository
50-
ghClient GitHubClient
51-
additionalMessage string
52-
commitMessage string
53-
prType string
45+
cfg *config.Config
46+
state *config.LibrarianState
47+
repo gitrepo.Repository
48+
ghClient GitHubClient
49+
idToCommits map[string]string
50+
failedLibraries []string
51+
commitMessage string
52+
prType string
5453
}
5554

5655
type commandRunner struct {
@@ -251,7 +250,7 @@ func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string)
251250
for _, srcRoot := range library.SourceRoots {
252251
dstPath := filepath.Join(dest, srcRoot)
253252
srcPath := filepath.Join(src, srcRoot)
254-
files, err := getDirectoryFilesnames(srcPath)
253+
files, err := getDirectoryFilenames(srcPath)
255254
if err != nil {
256255
return err
257256
}
@@ -267,7 +266,7 @@ func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string)
267266
return nil
268267
}
269268

270-
func getDirectoryFilesnames(dir string) ([]string, error) {
269+
func getDirectoryFilenames(dir string) ([]string, error) {
271270
if _, err := os.Stat(dir); err != nil {
272271
// Skip dirs that don't exist
273272
if os.IsNotExist(err) {
@@ -310,27 +309,6 @@ func copyLibrary(dst, src string, library *config.LibraryState) error {
310309
return nil
311310
}
312311

313-
func coerceLibraryChanges(commits []*conventionalcommits.ConventionalCommit) []*config.Change {
314-
changes := make([]*config.Change, 0)
315-
for _, commit := range commits {
316-
clNum := ""
317-
if cl, ok := commit.Footers[KeyClNum]; ok {
318-
clNum = cl
319-
}
320-
321-
changeType := getChangeType(commit)
322-
changes = append(changes, &config.Change{
323-
Type: changeType,
324-
Subject: commit.Description,
325-
Body: commit.Body,
326-
ClNum: clNum,
327-
CommitHash: commit.SHA,
328-
})
329-
}
330-
331-
return changes
332-
}
333-
334312
// commitAndPush creates a commit and push request to GitHub for the generated
335313
// changes.
336314
// It uses the GitHub client to create a PR with the specified branch, title, and
@@ -354,15 +332,11 @@ func commitAndPush(ctx context.Context, info *commitInfo) error {
354332

355333
datetimeNow := formatTimestamp(time.Now())
356334
branch := fmt.Sprintf("librarian-%s", datetimeNow)
357-
slog.Info("Creating branch", slog.String("branch", branch))
358335
if err := repo.CreateBranchAndCheckout(branch); err != nil {
359336
return err
360337
}
361338

362-
// TODO: get correct language for message (https://github.com/googleapis/librarian/issues/885)
363-
commitMessage := info.commitMessage
364-
slog.Info("Committing", "message", commitMessage)
365-
if err := repo.Commit(commitMessage); err != nil {
339+
if err := repo.Commit(info.commitMessage); err != nil {
366340
return err
367341
}
368342

@@ -382,14 +356,29 @@ func commitAndPush(ctx context.Context, info *commitInfo) error {
382356
}
383357

384358
title := fmt.Sprintf("Librarian %s pull request: %s", info.prType, datetimeNow)
385-
slog.Info("Creating pull request", slog.String("branch", branch), slog.String("title", title))
386-
if _, err = info.ghClient.CreatePullRequest(ctx, gitHubRepo, branch, cfg.Branch, title, commitMessage); err != nil {
359+
prBody, err := createPRBody(info)
360+
if err != nil {
361+
return fmt.Errorf("failed to create pull request body: %w", err)
362+
}
363+
364+
if _, err = info.ghClient.CreatePullRequest(ctx, gitHubRepo, branch, cfg.Branch, title, prBody); err != nil {
387365
return fmt.Errorf("failed to create pull request: %w", err)
388366
}
389367

390368
return nil
391369
}
392370

371+
func createPRBody(info *commitInfo) (string, error) {
372+
switch info.prType {
373+
case generate:
374+
return formatGenerationPRBody(info.repo, info.state, info.idToCommits, info.failedLibraries)
375+
case release:
376+
return formatReleaseNotes(info.repo, info.state)
377+
default:
378+
return "", fmt.Errorf("unrecognized pull request type: %s", info.prType)
379+
}
380+
}
381+
393382
func copyFile(dst, src string) (err error) {
394383
sourceFile, err := os.Open(src)
395384
if err != nil {

‎internal/librarian/command_test.go‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,8 @@ func TestCommitAndPush(t *testing.T) {
10891089
name string
10901090
setupMockRepo func(t *testing.T) gitrepo.Repository
10911091
setupMockClient func(t *testing.T) GitHubClient
1092+
state *config.LibrarianState
1093+
prType string
10921094
commit bool
10931095
push bool
10941096
wantErr bool
@@ -1107,6 +1109,7 @@ func TestCommitAndPush(t *testing.T) {
11071109
setupMockClient: func(t *testing.T) GitHubClient {
11081110
return nil
11091111
},
1112+
prType: "generate",
11101113
},
11111114
{
11121115
name: "create a commit",
@@ -1128,6 +1131,7 @@ func TestCommitAndPush(t *testing.T) {
11281131
createdPR: &github.PullRequestMetadata{Number: 123, Repo: &github.Repository{Owner: "test-owner", Name: "test-repo"}},
11291132
}
11301133
},
1134+
prType: "generate",
11311135
commit: true,
11321136
},
11331137
{
@@ -1150,7 +1154,9 @@ func TestCommitAndPush(t *testing.T) {
11501154
createdPR: &github.PullRequestMetadata{Number: 123, Repo: &github.Repository{Owner: "test-owner", Name: "test-repo"}},
11511155
}
11521156
},
1153-
push: true,
1157+
state: &config.LibrarianState{},
1158+
prType: "generate",
1159+
push: true,
11541160
},
11551161
{
11561162
name: "No GitHub Remote",
@@ -1166,6 +1172,7 @@ func TestCommitAndPush(t *testing.T) {
11661172
setupMockClient: func(t *testing.T) GitHubClient {
11671173
return nil
11681174
},
1175+
prType: "generate",
11691176
push: true,
11701177
wantErr: true,
11711178
expectedErrMsg: "could not find an 'origin' remote",
@@ -1186,6 +1193,7 @@ func TestCommitAndPush(t *testing.T) {
11861193
setupMockClient: func(t *testing.T) GitHubClient {
11871194
return nil
11881195
},
1196+
prType: "generate",
11891197
push: true,
11901198
wantErr: true,
11911199
expectedErrMsg: "mock add all error",
@@ -1210,6 +1218,7 @@ func TestCommitAndPush(t *testing.T) {
12101218
setupMockClient: func(t *testing.T) GitHubClient {
12111219
return nil
12121220
},
1221+
prType: "generate",
12131222
push: true,
12141223
wantErr: true,
12151224
expectedErrMsg: "create branch error",
@@ -1234,6 +1243,7 @@ func TestCommitAndPush(t *testing.T) {
12341243
setupMockClient: func(t *testing.T) GitHubClient {
12351244
return nil
12361245
},
1246+
prType: "generate",
12371247
push: true,
12381248
wantErr: true,
12391249
expectedErrMsg: "commit error",
@@ -1258,10 +1268,36 @@ func TestCommitAndPush(t *testing.T) {
12581268
setupMockClient: func(t *testing.T) GitHubClient {
12591269
return nil
12601270
},
1271+
prType: "generate",
12611272
push: true,
12621273
wantErr: true,
12631274
expectedErrMsg: "push error",
12641275
},
1276+
{
1277+
name: "Create PR body error",
1278+
setupMockRepo: func(t *testing.T) gitrepo.Repository {
1279+
remote := git.NewRemote(memory.NewStorage(), &gogitConfig.RemoteConfig{
1280+
Name: "origin",
1281+
URLs: []string{"https://github.com/googleapis/librarian.git"},
1282+
})
1283+
1284+
status := make(git.Status)
1285+
status["file.txt"] = &git.FileStatus{Worktree: git.Modified}
1286+
return &MockRepository{
1287+
Dir: t.TempDir(),
1288+
AddAllStatus: status,
1289+
RemotesValue: []*git.Remote{remote},
1290+
}
1291+
},
1292+
setupMockClient: func(t *testing.T) GitHubClient {
1293+
return &mockGitHubClient{}
1294+
},
1295+
state: &config.LibrarianState{},
1296+
prType: "random",
1297+
push: true,
1298+
wantErr: true,
1299+
expectedErrMsg: "failed to create pull request body",
1300+
},
12651301
{
12661302
name: "Create pull request error",
12671303
setupMockRepo: func(t *testing.T) gitrepo.Repository {
@@ -1283,6 +1319,8 @@ func TestCommitAndPush(t *testing.T) {
12831319
createPullRequestErr: errors.New("create pull request error"),
12841320
}
12851321
},
1322+
state: &config.LibrarianState{},
1323+
prType: "generate",
12861324
push: true,
12871325
wantErr: true,
12881326
expectedErrMsg: "failed to create pull request",
@@ -1303,7 +1341,8 @@ func TestCommitAndPush(t *testing.T) {
13031341
setupMockClient: func(t *testing.T) GitHubClient {
13041342
return nil
13051343
},
1306-
push: true,
1344+
prType: "generate",
1345+
push: true,
13071346
},
13081347
} {
13091348
t.Run(test.name, func(t *testing.T) {
@@ -1315,11 +1354,11 @@ func TestCommitAndPush(t *testing.T) {
13151354
}
13161355
commitInfo := &commitInfo{
13171356
cfg: localConfig,
1318-
state: nil,
1357+
state: test.state,
13191358
repo: repo,
13201359
ghClient: client,
13211360
commitMessage: "",
1322-
prType: generate,
1361+
prType: test.prType,
13231362
}
13241363
err := commitAndPush(context.Background(), commitInfo)
13251364

0 commit comments

Comments
 (0)