From 9ee23fcee4fd8bd19f2af1308eb3c0413789e4e0 Mon Sep 17 00:00:00 2001 From: Thomas Vilte Date: Wed, 29 Jul 2026 18:35:38 -0300 Subject: [PATCH] feat(release): Add merged pull requests to release notes --- internal/commands/release/formatter.go | 14 +++++++++ internal/i18n/locales/active.en.toml | 5 ++-- internal/i18n/locales/active.es.toml | 5 ++-- internal/services/release_service.go | 28 ++++++++++++++++-- internal/services/release_service_test.go | 35 +++++++++++++++++++++++ internal/vcs/github/client.go | 5 +++- internal/vcs/github/client_test.go | 10 ++++--- 7 files changed, 91 insertions(+), 11 deletions(-) diff --git a/internal/commands/release/formatter.go b/internal/commands/release/formatter.go index 45a2d58..5912597 100644 --- a/internal/commands/release/formatter.go +++ b/internal/commands/release/formatter.go @@ -74,6 +74,20 @@ func FormatReleaseMarkdown(release *models.Release, notes *models.ReleaseNotes, } } + if len(release.MergedPRs) > 0 { + md.WriteString("## ") + md.WriteString(trans.GetMessage("release.md_pull_requests", 0, nil)) + md.WriteString("\n\n") + for _, pr := range release.MergedPRs { + if pr.URL != "" { + md.WriteString(fmt.Sprintf("- [#%d](%s) %s (by @%s)\n", pr.Number, pr.URL, pr.Title, pr.Author)) + } else { + md.WriteString(fmt.Sprintf("- #%d %s (by @%s)\n", pr.Number, pr.Title, pr.Author)) + } + } + md.WriteString("\n") + } + if len(release.Contributors) > 0 { md.WriteString("## ") md.WriteString(trans.GetMessage("release.md_contributors", 0, nil)) diff --git a/internal/i18n/locales/active.en.toml b/internal/i18n/locales/active.en.toml index 3a2593f..feee75f 100644 --- a/internal/i18n/locales/active.en.toml +++ b/internal/i18n/locales/active.en.toml @@ -405,10 +405,11 @@ md_version = "Version" md_previous = "Previous Version" md_summary = "Summary" md_highlights = "Highlights" -md_contributors = "👥 Contributors" +md_contributors = "Contributors" new_contributors = "Welcome {{.Count}} new contributors!" all_contributors = "All contributors for this release:" -md_stats = "📊 Statistics" +md_pull_requests = "Pull Requests" +md_stats = "Statistics" files_changed = "Files changed" insertions = "Lines added" deletions = "Lines deleted" diff --git a/internal/i18n/locales/active.es.toml b/internal/i18n/locales/active.es.toml index a2e5e73..c215c91 100644 --- a/internal/i18n/locales/active.es.toml +++ b/internal/i18n/locales/active.es.toml @@ -357,10 +357,11 @@ empty_body_warning = "⚠️ Esta release no tiene descripción. Se usará un t template_warning = "Esta release no tiene descripción en GitHub" template_tip = "Tip: Podés regenerar las notas con 'matecommit release generate'" template_changes = "Cambios" -md_contributors = "👥 Contributors" +md_contributors = "Contributors" new_contributors = "¡Damos la bienvenida a {Count} nuevos contributors!" all_contributors = "Todos los contributors de este release:" -md_stats = "📊 Estadísticas" +md_pull_requests = "Pull Requests" +md_stats = "Estadísticas" files_changed = "Archivos modificados" insertions = "Líneas agregadas" deletions = "Líneas eliminadas" diff --git a/internal/services/release_service.go b/internal/services/release_service.go index f9ab7fa..764d4fd 100644 --- a/internal/services/release_service.go +++ b/internal/services/release_service.go @@ -278,36 +278,60 @@ func (s *ReleaseService) EnrichReleaseContext(ctx context.Context, release *mode return domainErrors.ErrConfigMissing } + // release.Version is the prospective next version — it isn't a real tag + // yet during preview (and possibly not even during create, before the + // tag is pushed), so comparisons need the actual current branch tip + // instead. It points at the same commit a freshly created tag would. + headRef := release.Version + if s.git != nil { + if branch, err := s.git.GetCurrentBranch(ctx); err == nil { + headRef = branch + } else { + log.Warn("failed to resolve current branch for release comparisons, falling back to version string", + "error", err) + } + } + if issues, err := s.vcsClient.GetClosedIssuesBetweenTags(ctx, release.PreviousVersion, release.Version); err == nil { release.ClosedIssues = issues log.Debug("closed issues fetched", "count", len(issues)) + } else { + log.Warn("failed to fetch closed issues", "error", err) } if prs, err := s.vcsClient.GetMergedPRsBetweenTags(ctx, release.PreviousVersion, release.Version); err == nil { release.MergedPRs = prs log.Debug("merged PRs fetched", "count", len(prs)) + } else { + log.Warn("failed to fetch merged PRs", "error", err) } - if contributors, err := s.vcsClient.GetContributorsBetweenTags(ctx, release.PreviousVersion, release.Version); err == nil { + if contributors, err := s.vcsClient.GetContributorsBetweenTags(ctx, release.PreviousVersion, headRef); err == nil { release.Contributors = contributors release.NewContributors = contributors log.Debug("contributors fetched", "count", len(contributors)) + } else { + log.Warn("failed to fetch contributors", "error", err) } - if stats, err := s.vcsClient.GetFileStatsBetweenTags(ctx, release.PreviousVersion, release.Version); err == nil { + if stats, err := s.vcsClient.GetFileStatsBetweenTags(ctx, release.PreviousVersion, headRef); err == nil { release.FileStats = *stats log.Debug("file stats fetched", "files_changed", stats.FilesChanged, "insertions", stats.Insertions, "deletions", stats.Deletions) + } else { + log.Warn("failed to fetch file stats", "error", err) } if deps, err := s.analyzeDependencyChanges(ctx, release); err == nil { release.Dependencies = deps log.Debug("dependencies analyzed") + } else { + log.Warn("failed to analyze dependency changes", "error", err) } log.Info("release context enriched successfully") diff --git a/internal/services/release_service_test.go b/internal/services/release_service_test.go index c23233a..abaff90 100644 --- a/internal/services/release_service_test.go +++ b/internal/services/release_service_test.go @@ -483,6 +483,41 @@ func TestReleaseService_EnrichReleaseContext(t *testing.T) { assert.Equal(t, 3, release.FileStats.FilesChanged) mockVCS.AssertExpectations(t) }) + + t.Run("compares contributors and file stats against the current branch, not the prospective version", func(t *testing.T) { + // release.Version is a version that hasn't been tagged yet (e.g. + // during preview), so GitHub can't compare against it directly — + // the current branch tip points at the same commit a real tag + // would, and always exists. + mockGit := new(testutil.MockGitService) + mockVCS := new(testutil.MockVCSClient) + service := NewReleaseService(mockGit, WithReleaseVCSClient(mockVCS)) + + release := &models.Release{ + PreviousVersion: "v1.0.0", + Version: "v1.1.0", + } + + mockGit.On("GetCurrentBranch", mock.Anything).Return("master", nil) + mockVCS.On("GetClosedIssuesBetweenTags", mock.Anything, "v1.0.0", "v1.1.0"). + Return([]models.Issue{}, nil) + mockVCS.On("GetMergedPRsBetweenTags", mock.Anything, "v1.0.0", "v1.1.0"). + Return([]models.PullRequest{}, nil) + mockVCS.On("GetContributorsBetweenTags", mock.Anything, "v1.0.0", "master"). + Return([]string{"user1"}, nil) + mockVCS.On("GetFileStatsBetweenTags", mock.Anything, "v1.0.0", "master"). + Return(&models.FileStatistics{FilesChanged: 7}, nil) + mockVCS.On("GetFileAtTag", mock.Anything, mock.Anything, mock.Anything). + Return("", errors.New("not found")) + + err := service.EnrichReleaseContext(context.Background(), release) + + assert.NoError(t, err) + assert.Len(t, release.Contributors, 1) + assert.Equal(t, 7, release.FileStats.FilesChanged) + mockGit.AssertExpectations(t) + mockVCS.AssertExpectations(t) + }) } func TestReleaseService_UpdateAppVersion(t *testing.T) { diff --git a/internal/vcs/github/client.go b/internal/vcs/github/client.go index b7bab36..bc9799b 100644 --- a/internal/vcs/github/client.go +++ b/internal/vcs/github/client.go @@ -536,7 +536,10 @@ func (ghc *GitHubClient) GetMergedPRsBetweenTags(ctx context.Context, previousTa } for _, pr := range prs { - if pr.GetMerged() && pr.GetMergedAt().After(prevRelease.GetCreatedAt().Time) { + // The list endpoint never populates the "merged" boolean field + // (only the single-PR endpoint does) — merged_at is the only + // reliable signal here, and a zero value means "not merged". + if !pr.GetMergedAt().IsZero() && pr.GetMergedAt().After(prevRelease.GetCreatedAt().Time) { labels := make([]string, 0, len(pr.Labels)) for _, label := range pr.Labels { labels = append(labels, label.GetName()) diff --git a/internal/vcs/github/client_test.go b/internal/vcs/github/client_test.go index 60a3bbb..5b490f0 100644 --- a/internal/vcs/github/client_test.go +++ b/internal/vcs/github/client_test.go @@ -903,25 +903,28 @@ func TestGitHubClient_GetMergedPRsBetweenTags(t *testing.T) { CreatedAt: &prevReleaseDate, } + // The "List pull requests" endpoint never populates the "merged" + // boolean field (only the single-PR endpoint does) — real API + // responses only ever carry merged_at, so none of these set Merged. pr1 := &github.PullRequest{ Number: github.Ptr(1), Title: github.Ptr("PR 1"), Body: github.Ptr("Description 1"), User: &github.User{Login: github.Ptr("user1")}, - Merged: github.Ptr(true), MergedAt: &github.Timestamp{Time: mergedTime1}, HTMLURL: github.Ptr("url1"), Labels: []*github.Label{{Name: github.Ptr("bug")}}, } + // Closed but never merged: no MergedAt at all, matching how a real + // closed-without-merging PR looks from the list endpoint. pr2 := &github.PullRequest{ Number: github.Ptr(2), - Merged: github.Ptr(false), } + // Merged, but before the previous release — must be excluded by date. pr3 := &github.PullRequest{ Number: github.Ptr(3), - Merged: github.Ptr(true), MergedAt: &github.Timestamp{Time: mergedTime2}, } @@ -962,7 +965,6 @@ func TestGitHubClient_GetMergedPRsBetweenTags(t *testing.T) { mergedTime := time.Now() pr1 := &github.PullRequest{ Number: github.Ptr(1), - Merged: github.Ptr(true), MergedAt: &github.Timestamp{Time: mergedTime}, }