Skip to content

Commit 2d2ca1c

Browse files
pedronauckpedronauck
committed
refactor(repo): Improve performance
1 parent 4d9f15d commit 2d2ca1c

File tree

7 files changed

+85
-125
lines changed

7 files changed

+85
-125
lines changed

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ require (
5050
go.yaml.in/yaml/v3 v3.0.4 // indirect
5151
golang.org/x/crypto v0.37.0 // indirect
5252
golang.org/x/net v0.39.0 // indirect
53+
golang.org/x/sync v0.17.0 // indirect
5354
golang.org/x/sys v0.32.0 // indirect
5455
golang.org/x/text v0.28.0 // indirect
5556
gopkg.in/warnings.v0 v0.1.2 // indirect

‎go.sum‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,8 @@ golang.org/x/net v0.39.0 h1:ZCu7HMWDxpXpaiKdhzIfaltL9Lp31x/3fCP11bc6/fY=
129129
golang.org/x/net v0.39.0/go.mod h1:X7NRbYVEA+ewNkCNyJ513WmMdQ3BineSwVtN2zD/d+E=
130130
golang.org/x/oauth2 v0.31.0 h1:8Fq0yVZLh4j4YA47vHKFTa9Ew5XIrCP8LC6UeNZnLxo=
131131
golang.org/x/oauth2 v0.31.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA=
132+
golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug=
133+
golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
132134
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
133135
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
134136
golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

‎internal/orchestrator/pr_release.go‎

Lines changed: 33 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/sethvargo/go-retry"
1717
"github.com/spf13/afero"
1818
"go.uber.org/zap"
19+
"golang.org/x/sync/errgroup"
1920
)
2021

2122
// PRReleaseConfig contains configuration for PR release workflow.
@@ -374,8 +375,7 @@ func (o *PRReleaseOrchestrator) buildAndExecuteWorkflow(
374375
o.addCheckChangesStep(saga, cfg, compensator, wctx)
375376
o.addCalculateVersionStep(saga, cfg, compensator, wctx)
376377
o.addCreateBranchStep(saga, cfg, compensator, wctx, originalBranch)
377-
o.addUpdatePackagesStep(saga, compensator, wctx)
378-
o.addGenerateChangelogStep(saga, compensator, wctx)
378+
o.addPrepareReleaseArtifactsStep(saga, compensator, wctx)
379379
o.addCommitChangesStep(saga, compensator, wctx)
380380
o.addPushBranchStep(saga, cfg, compensator, wctx)
381381
o.addCreatePRStep(saga, cfg, compensator, wctx)
@@ -397,6 +397,7 @@ type workflowContext struct {
397397
latestTag string
398398
prNumber int
399399
createdInSession bool
400+
changelog string
400401
}
401402

402403
// Workflow step methods
@@ -608,56 +609,50 @@ func (o *PRReleaseOrchestrator) logBranchStatus(
608609
}
609610
}
610611

611-
func (o *PRReleaseOrchestrator) addUpdatePackagesStep(
612+
func (o *PRReleaseOrchestrator) addPrepareReleaseArtifactsStep(
612613
saga *SagaExecutor,
613614
compensator *CompensatingActions,
614615
wctx *workflowContext,
615616
) {
616617
saga.AddStep(SagaStep{
617-
Name: "Update Package Versions",
618+
Name: "Prepare Release Artifacts",
618619
Type: domain.OperationTypeUpdatePackages,
619620
Execute: func(ctx context.Context) (map[string]any, error) {
620621
if wctx.version == "" {
621622
return map[string]any{"skip": true}, nil
622623
}
623-
o.logger(ctx).Info("Updating package versions", zap.String("version", wctx.version))
624-
if err := o.updatePackageVersions(ctx, wctx.version); err != nil {
625-
o.logger(ctx).Error("Failed to update package versions", zap.Error(err))
626-
return nil, fmt.Errorf("failed to update package versions: %w", err)
624+
o.logger(ctx).Info("Preparing release artifacts", zap.String("version", wctx.version))
625+
g, gctx := errgroup.WithContext(ctx)
626+
var changelog string
627+
g.Go(func() error {
628+
o.logger(gctx).Info("Updating package versions", zap.String("version", wctx.version))
629+
if err := o.updatePackageVersions(gctx, wctx.version); err != nil {
630+
o.logger(gctx).Error("Failed to update package versions", zap.Error(err))
631+
return fmt.Errorf("failed to update package versions: %w", err)
632+
}
633+
o.logger(gctx).Info("Updated package versions", zap.String("version", wctx.version))
634+
return nil
635+
})
636+
g.Go(func() error {
637+
o.logger(gctx).Info("Generating changelog", zap.String("version", wctx.version))
638+
var err error
639+
changelog, err = o.generateChangelog(gctx, wctx.version, "unreleased")
640+
if err != nil {
641+
o.logger(gctx).Error("Failed to generate changelog", zap.Error(err))
642+
return fmt.Errorf("failed to generate changelog: %w", err)
643+
}
644+
o.logger(gctx).Info("Generated changelog", zap.String("version", wctx.version))
645+
return nil
646+
})
647+
if err := g.Wait(); err != nil {
648+
return nil, err
627649
}
628-
o.logger(ctx).Info("Updated package versions", zap.String("version", wctx.version))
650+
wctx.changelog = changelog
651+
o.logger(ctx).Info("Release artifacts prepared successfully", zap.String("version", wctx.version))
629652
return map[string]any{
630653
"modified_files": []string{
631654
"package.json",
632655
"package-lock.json",
633-
},
634-
}, nil
635-
},
636-
Compensate: compensator.RestoreFiles,
637-
})
638-
}
639-
640-
func (o *PRReleaseOrchestrator) addGenerateChangelogStep(
641-
saga *SagaExecutor,
642-
compensator *CompensatingActions,
643-
wctx *workflowContext,
644-
) {
645-
saga.AddStep(SagaStep{
646-
Name: "Generate Changelog",
647-
Type: domain.OperationTypeGenerateChangelog,
648-
Execute: func(ctx context.Context) (map[string]any, error) {
649-
if wctx.version == "" {
650-
return map[string]any{"skip": true}, nil
651-
}
652-
o.logger(ctx).Info("Generating changelog", zap.String("version", wctx.version))
653-
changelog, err := o.generateChangelog(ctx, wctx.version, "unreleased")
654-
if err != nil {
655-
o.logger(ctx).Error("Failed to generate changelog", zap.Error(err))
656-
return nil, fmt.Errorf("failed to generate changelog: %w", err)
657-
}
658-
o.logger(ctx).Info("Generated changelog", zap.String("version", wctx.version))
659-
return map[string]any{
660-
"modified_files": []string{
661656
"CHANGELOG.md",
662657
"RELEASE_NOTES.md",
663658
},
@@ -744,11 +739,7 @@ func (o *PRReleaseOrchestrator) addCreatePRStep(
744739
return map[string]any{"skip": true}, nil
745740
}
746741
o.logger(ctx).Info("Preparing pull request", zap.String("version", wctx.version))
747-
changelog, err := o.generateChangelog(ctx, wctx.version, "unreleased")
748-
if err != nil {
749-
o.logger(ctx).Error("Failed to generate changelog for PR", zap.Error(err))
750-
return nil, fmt.Errorf("failed to get changelog for PR: %w", err)
751-
}
742+
changelog := wctx.changelog
752743
ver, err := domain.NewVersion(wctx.version)
753744
if err != nil {
754745
o.logger(ctx).Error("Failed to parse version", zap.String("version", wctx.version), zap.Error(err))

‎internal/orchestrator/pr_release_test.go‎

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
4343
// Setup expectations for createReleaseBranch
4444
branchName := "release/v1.1.0"
4545
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
46-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
4746
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
4847

4948
// Setup expectations for generateChangelog
@@ -59,8 +58,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
5958
gitRepo.On("AddFiles", mock.Anything, "package-lock.json").Return(nil).Once()
6059
// tools/* updates removed
6160
gitRepo.On("Commit", mock.Anything, "ci(release): prepare release v1.1.0").Return(nil).Once()
62-
63-
// Setup expectations for push and PR creation
6461
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
6562
githubRepo.On("CreateOrUpdatePR", mock.Anything, branchName, "main", "ci(release): Release v1.1.0",
6663
mock.MatchedBy(func(body string) bool {
@@ -148,7 +145,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
148145
// Setup remaining expectations for forced release
149146
branchName := "release/v1.0.1"
150147
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
151-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
152148
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
153149

154150
changelog := "## v1.0.1\n\n### Maintenance\n- Forced release"
@@ -256,7 +252,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
256252

257253
branchName := "release/v1.1.0"
258254
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
259-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
260255
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
261256

262257
// Fail on changelog generation (use mock.Anything for context)
@@ -298,7 +293,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
298293

299294
branchName := "release/v1.1.0"
300295
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
301-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Times(2) // Once for branch, once after commit
302296
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
303297

304298
changelog := "## v1.1.0\n\n### Features\n- New feature"
@@ -307,6 +301,7 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
307301
gitRepo.On("ConfigureUser", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
308302
gitRepo.On("AddFiles", mock.Anything, mock.Anything).Return(nil).Times(3)
309303
gitRepo.On("Commit", mock.Anything, mock.Anything).Return(nil).Once()
304+
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
310305

311306
// Fail on PR creation (use mock.Anything for context)
312307
// Note: The retry might not be happening for non-retryable errors
@@ -347,7 +342,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
347342

348343
branchName := "release/v1.1.0"
349344
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
350-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Times(2)
351345
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
352346

353347
changelog := "## v1.1.0\n\n### Features\n- New feature"
@@ -356,6 +350,7 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
356350
gitRepo.On("ConfigureUser", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
357351
gitRepo.On("AddFiles", mock.Anything, mock.Anything).Return(nil).Times(3)
358352
gitRepo.On("Commit", mock.Anything, mock.Anything).Return(nil).Once()
353+
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
359354

360355
orch := NewPRReleaseOrchestrator(gitRepo, githubRepo, fsRepo, cliffSvc, npmSvc)
361356
cfg := PRReleaseConfig{
@@ -435,7 +430,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
435430

436431
branchName := "release/v0.1.0"
437432
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
438-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Times(2)
439433
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
440434

441435
changelog := "## v0.1.0\n\n### Features\n- Initial release"
@@ -444,6 +438,7 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
444438
gitRepo.On("ConfigureUser", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
445439
gitRepo.On("AddFiles", mock.Anything, mock.Anything).Return(nil).Times(3)
446440
gitRepo.On("Commit", mock.Anything, mock.Anything).Return(nil).Once()
441+
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
447442
githubRepo.On("CreateOrUpdatePR", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
448443
Return(nil).
449444
Once()
@@ -518,7 +513,6 @@ func TestPRReleaseOrchestrator_Execute(t *testing.T) {
518513

519514
branchName := "release/v1.1.0"
520515
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
521-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
522516
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
523517

524518
changelog := "## v1.1.0\n\n### Features\n- New feature"
@@ -615,7 +609,6 @@ func TestPRReleaseOrchestrator_RollbackOnFailure(t *testing.T) {
615609
gitRepo.On("ListLocalBranches", mock.Anything).Return([]string{"main"}, nil).Once()
616610
// Once for create, once during rollback check
617611
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
618-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
619612
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
620613

621614
// Setup expectations for updatePackages step - successful
@@ -694,7 +687,6 @@ func TestPRReleaseOrchestrator_RollbackOnFailure(t *testing.T) {
694687
// Mock ListLocalBranches to return branches WITHOUT the target branch (so it gets created)
695688
gitRepo.On("ListLocalBranches", mock.Anything).Return([]string{"main"}, nil).Once()
696689
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
697-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Times(2)
698690
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
699691

700692
changelog := "## v1.1.0\n\n### Features\n- New feature"
@@ -706,6 +698,7 @@ func TestPRReleaseOrchestrator_RollbackOnFailure(t *testing.T) {
706698
gitRepo.On("ConfigureUser", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
707699
gitRepo.On("AddFiles", mock.Anything, mock.Anything).Return(nil).Times(3)
708700
gitRepo.On("Commit", mock.Anything, mock.Anything).Return(nil).Once()
701+
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
709702

710703
// PR creation fails
711704
githubRepo.On("CreateOrUpdatePR", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
@@ -793,7 +786,6 @@ func TestPRReleaseOrchestrator_RollbackOnFailure(t *testing.T) {
793786
gitRepo.On("ListLocalBranches", mock.Anything).Return([]string{"main"}, nil).Once()
794787
gitRepo.On("GetCurrentBranch", mock.Anything).Return("main", nil).Once()
795788
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
796-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
797789
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
798790

799791
// Fail on changelog generation
@@ -857,7 +849,6 @@ func TestPRReleaseOrchestrator_DisabledRollback(t *testing.T) {
857849

858850
branchName := "release/v1.1.0"
859851
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
860-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Times(2)
861852
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
862853

863854
changelog := "## v1.1.0\n\n### Features\n- New feature"
@@ -866,6 +857,7 @@ func TestPRReleaseOrchestrator_DisabledRollback(t *testing.T) {
866857
gitRepo.On("ConfigureUser", mock.Anything, mock.Anything, mock.Anything).Return(nil).Once()
867858
gitRepo.On("AddFiles", mock.Anything, mock.Anything).Return(nil).Times(3)
868859
gitRepo.On("Commit", mock.Anything, mock.Anything).Return(nil).Once()
860+
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
869861

870862
githubRepo.On("CreateOrUpdatePR", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
871863
Return(nil).
@@ -906,7 +898,6 @@ func TestPRReleaseOrchestrator_DisabledRollback(t *testing.T) {
906898

907899
branchName := "release/v1.1.0"
908900
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
909-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
910901
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
911902

912903
// Fail on changelog generation
@@ -945,7 +936,6 @@ func TestPRReleaseOrchestrator_prepareRelease(t *testing.T) {
945936
// Setup branch creation expectations (use mock.Anything for context)
946937
branchName := "release/v1.0.0"
947938
gitRepo.On("CreateBranch", mock.Anything, branchName).Return(nil).Once()
948-
gitRepo.On("PushBranch", mock.Anything, branchName).Return(nil).Once()
949939
gitRepo.On("CheckoutBranch", mock.Anything, branchName).Return(nil).Once()
950940

951941
orch := NewPRReleaseOrchestrator(gitRepo, githubRepo, fsRepo, cliffSvc, npmSvc)

0 commit comments

Comments
 (0)