refactor: move commit analyzing out of forge

changelog.go

@@ -90,12 +90,11 @@ func UpdateChangelogFile(wt *git.Worktree, newEntry string) error {
 	return nil
 }
 
-func NewChangelogEntry(changesets []Changeset, version, link, prefix, suffix string) (string, error) {
+func NewChangelogEntry(commits []AnalyzedCommit, version, link, prefix, suffix string) (string, error) {
 	features := make([]AnalyzedCommit, 0)
 	fixes := make([]AnalyzedCommit, 0)
 
-	for _, changeset := range changesets {
-		for _, commit := range changeset.ChangelogEntries {
+	for _, commit := range commits {
 		switch commit.Type {
 		case "feat":
 			features = append(features, commit)
@@ -103,7 +102,6 @@ func NewChangelogEntry(changesets []Changeset, version, link, prefix, suffix str
 			fixes = append(fixes, commit)
 		}
 	}
-	}
 
 	var changelog bytes.Buffer
 	err := changelogTemplate.Execute(&changelog, map[string]any{

changelog_test.go

@@ -96,7 +96,7 @@ func TestUpdateChangelogFile(t *testing.T) {
 
 func Test_NewChangelogEntry(t *testing.T) {
 	type args struct {
-		changesets []Changeset
+		analyzedCommits []AnalyzedCommit
 		version         string
 		link            string
 		prefix          string
@@ -111,7 +111,7 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "empty",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{}}},
+				analyzedCommits: []AnalyzedCommit{},
 				version:         "1.0.0",
 				link:            "https://example.com/1.0.0",
 			},
@@ -121,13 +121,13 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "single feature",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{
+				analyzedCommits: []AnalyzedCommit{
 					{
 						Commit:      Commit{},
 						Type:        "feat",
 						Description: "Foobar!",
 					},
-				}}},
+				},
 				version: "1.0.0",
 				link:    "https://example.com/1.0.0",
 			},
@@ -137,13 +137,13 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "single fix",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{
+				analyzedCommits: []AnalyzedCommit{
 					{
 						Commit:      Commit{},
 						Type:        "fix",
 						Description: "Foobar!",
 					},
-				}}},
+				},
 				version: "1.0.0",
 				link:    "https://example.com/1.0.0",
 			},
@@ -153,7 +153,7 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "multiple commits with scopes",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{
+				analyzedCommits: []AnalyzedCommit{
 					{
 						Commit:      Commit{},
 						Type:        "feat",
@@ -176,7 +176,7 @@ func Test_NewChangelogEntry(t *testing.T) {
 						Description: "So sad!",
 						Scope:       ptr("sad"),
 					},
-				}}},
+				},
 				version: "1.0.0",
 				link:    "https://example.com/1.0.0",
 			},
@@ -196,13 +196,13 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "prefix",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{
+				analyzedCommits: []AnalyzedCommit{
 					{
 						Commit:      Commit{},
 						Type:        "fix",
 						Description: "Foobar!",
 					},
-				}}},
+				},
 				version: "1.0.0",
 				link:    "https://example.com/1.0.0",
 				prefix:  "### Breaking Changes",
@@ -219,13 +219,13 @@ func Test_NewChangelogEntry(t *testing.T) {
 		{
 			name: "suffix",
 			args: args{
-				changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{
+				analyzedCommits: []AnalyzedCommit{
 					{
 						Commit:      Commit{},
 						Type:        "fix",
 						Description: "Foobar!",
 					},
-				}}},
+				},
 				version: "1.0.0",
 				link:    "https://example.com/1.0.0",
 				suffix:  "### Compatibility\n\nThis version is compatible with flux-compensator v2.2 - v2.9.",
@@ -245,7 +245,7 @@ This version is compatible with flux-compensator v2.2 - v2.9.
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := NewChangelogEntry(tt.args.changesets, tt.args.version, tt.args.link, tt.args.prefix, tt.args.suffix)
+			got, err := NewChangelogEntry(tt.args.analyzedCommits, tt.args.version, tt.args.link, tt.args.prefix, tt.args.suffix)
 			if !tt.wantErr(t, err) {
 				return
 			}

cmd/rp/cmd/run.go

@@ -59,7 +59,7 @@ func run(cmd *cobra.Command, _ []string) error {
 		})
 	}
 
-	releaserPleaser := rp.New(forge, logger, flagBranch)
+	releaserPleaser := rp.New(forge, logger, flagBranch, rp.NewConventionalCommitsParser(), rp.SemVerNextVersion)
 
 	return releaserPleaser.Run(ctx)
 }

commits.go

@@ -7,6 +7,19 @@ import (
 	"github.com/leodido/go-conventionalcommits/parser"
 )
 
+type Commit struct {
+	Hash    string
+	Message string
+
+	PullRequest *PullRequest
+}
+
+type PullRequest struct {
+	ID          int
+	Title       string
+	Description string
+}
+
 type AnalyzedCommit struct {
 	Commit
 	Type           string
@@ -34,7 +47,7 @@ func NewConventionalCommitsParser() *ConventionalCommitsParser {
 	}
 }
 
-func (c *ConventionalCommitsParser) AnalyzeCommits(commits []Commit) ([]AnalyzedCommit, error) {
+func (c *ConventionalCommitsParser) Analyze(commits []Commit) ([]AnalyzedCommit, error) {
 	analyzedCommits := make([]AnalyzedCommit, 0, len(commits))
 
 	for _, commit := range commits {

commits_test.go

@@ -111,7 +111,7 @@ func TestAnalyzeCommits(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			analyzedCommits, err := NewConventionalCommitsParser().AnalyzeCommits(tt.commits)
+			analyzedCommits, err := NewConventionalCommitsParser().Analyze(tt.commits)
 			if !tt.wantErr(t, err) {
 				return
 			}

forge.go

@@ -25,12 +25,6 @@ const (
 	GitHubLabelColor    = "dedede"
 )
 
-type Changeset struct {
-	URL              string
-	Identifier       string
-	ChangelogEntries []AnalyzedCommit
-}
-
 type Forge interface {
 	RepoURL() string
 	CloneURL() string
@@ -46,9 +40,6 @@ type Forge interface {
 	// function should return all commits.
 	CommitsSince(context.Context, *Tag) ([]Commit, error)
 
-	// Changesets looks up the Pull/Merge Requests for each commit, returning its parsed metadata.
-	Changesets(context.Context, []Commit) ([]Changeset, error)
-
 	// EnsureLabelsExist verifies that all desired labels are available on the repository. If labels are missing, they
 	// are created them.
 	EnsureLabelsExist(context.Context, []Label) error
@@ -184,10 +175,16 @@ func (g *GitHub) CommitsSince(ctx context.Context, tag *Tag) ([]Commit, error) {
 
 	var commits = make([]Commit, 0, len(repositoryCommits))
 	for _, ghCommit := range repositoryCommits {
-		commits = append(commits, Commit{
+		commit := Commit{
 			Hash:    ghCommit.GetSHA(),
 			Message: ghCommit.GetCommit().GetMessage(),
-		})
+		}
+		commit.PullRequest, err = g.prForCommit(ctx, commit)
+		if err != nil {
+			return nil, fmt.Errorf("failed to check for commit pull request: %w", err)
+		}
+
+		commits = append(commits, commit)
 	}
 
 	return commits, nil
@@ -272,15 +269,12 @@ func (g *GitHub) commitsSinceInit(ctx context.Context) ([]*github.RepositoryComm
 	return repositoryCommits, nil
 }
 
-func (g *GitHub) Changesets(ctx context.Context, commits []Commit) ([]Changeset, error) {
+func (g *GitHub) prForCommit(ctx context.Context, commit Commit) (*PullRequest, error) {
 	// We naively look up the associated PR for each commit through the "List pull requests associated with a commit"
 	// endpoint. This requires len(commits) requests.
 	// Using the "List pull requests" endpoint might be faster, as it allows us to fetch 100 arbitrary PRs per request,
 	// but worst case we need to look up all PRs made in the repository ever.
 
-	changesets := make([]Changeset, 0, len(commits))
-
-	for _, commit := range commits {
 	log := g.log.With("commit.hash", commit.Hash)
 	page := 1
 	var associatedPRs []*github.PullRequest
@@ -314,31 +308,10 @@ func (g *GitHub) Changesets(ctx context.Context, commits []Commit) ([]Changeset,
 		}
 	}
 	if pullrequest == nil {
-			log.Warn("did not find associated pull request, not considering it for changesets")
-			// No pull request was found for this commit, nothing to do here
-			// TODO: We could also return the minimal changeset for this commit, so at least it would come up in the changelog.
-			continue
+		return nil, nil
 	}
 
-		log = log.With("pullrequest.id", pullrequest.GetID())
-
-		// TODO: Parse PR description for overrides
-		changelogEntries, err := NewConventionalCommitsParser().AnalyzeCommits([]Commit{commit})
-		if err != nil {
-			log.Warn("unable to parse changelog entries", "error", err)
-			continue
-		}
-
-		if len(changelogEntries) > 0 {
-			changesets = append(changesets, Changeset{
-				URL:              pullrequest.GetHTMLURL(),
-				Identifier:       fmt.Sprintf("#%d", pullrequest.GetNumber()),
-				ChangelogEntries: changelogEntries,
-			})
-		}
-	}
-
-	return changesets, nil
+	return gitHubPRToPullRequest(pullrequest), nil
 }
 
 func (g *GitHub) EnsureLabelsExist(ctx context.Context, labels []Label) error {
@@ -578,6 +551,14 @@ func (g *GitHub) CreateRelease(ctx context.Context, commit Commit, title, change
 	return nil
 }
 
+func gitHubPRToPullRequest(pr *github.PullRequest) *PullRequest {
+	return &PullRequest{
+		ID:          pr.GetNumber(),
+		Title:       pr.GetTitle(),
+		Description: pr.GetBody(),
+	}
+}
+
 func gitHubPRToReleasePullRequest(pr *github.PullRequest) *ReleasePullRequest {
 	labels := make([]Label, 0, len(pr.Labels))
 	for _, label := range pr.Labels {

git.go

@@ -17,11 +17,6 @@ const (
 	GitRemoteName     = "origin"
 )
 
-type Commit struct {
-	Hash    string
-	Message string
-}
-
 type Tag struct {
 	Hash string
 	Name string

releasepr.go

@@ -31,6 +31,9 @@ func init() {
 	}
 }
 
+// ReleasePullRequest
+//
+// TODO: Reuse [PullRequest]
 type ReleasePullRequest struct {
 	ID          int
 	Title       string
@@ -58,7 +61,7 @@ func NewReleasePullRequest(head, branch, version, changelogEntry string) (*Relea
 type ReleaseOverrides struct {
 	Prefix string
 	Suffix string
-	// TODO: Doing the changelog for normal releases after previews requires to know about this while fetching the changesets
+	// TODO: Doing the changelog for normal releases after previews requires to know about this while fetching the commits
 	NextVersionType NextVersionType
 }
 

releaserpleaser.go

@@ -18,13 +18,17 @@ type ReleaserPleaser struct {
 	forge        Forge
 	logger       *slog.Logger
 	targetBranch string
+	commitParser CommitParser
+	nextVersion  VersioningStrategy
 }
 
-func New(forge Forge, logger *slog.Logger, targetBranch string) *ReleaserPleaser {
+func New(forge Forge, logger *slog.Logger, targetBranch string, commitParser CommitParser, versioningStrategy VersioningStrategy) *ReleaserPleaser {
 	return &ReleaserPleaser{
 		forge:        forge,
 		logger:       logger,
 		targetBranch: targetBranch,
+		commitParser: commitParser,
+		nextVersion:  versioningStrategy,
 	}
 }
 
@@ -154,12 +158,13 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error {
 
 	logger.InfoContext(ctx, "Found releasable commits", "length", len(releasableCommits))
 
-	changesets, err := rp.forge.Changesets(ctx, releasableCommits)
+	// TODO: Handle commit overrides
+	analyzedCommits, err := rp.commitParser.Analyze(releasableCommits)
 	if err != nil {
 		return err
 	}
 
-	logger.InfoContext(ctx, "Found changesets", "length", len(changesets))
+	logger.InfoContext(ctx, "Analyzed commits", "length", len(analyzedCommits))
 
 	rpBranch := fmt.Sprintf(PullRequestBranchFormat, rp.targetBranch)
 	rpBranchRef := plumbing.NewBranchReferenceName(rpBranch)
@@ -177,15 +182,15 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error {
 		logger.InfoContext(ctx, "found existing release pull request", "pr.id", pr.ID, "pr.title", pr.Title)
 	}
 
-	if len(changesets) == 0 {
+	if len(analyzedCommits) == 0 {
 		if pr != nil {
-			logger.InfoContext(ctx, "closing existing pull requests, no changesets available", "pr.id", pr.ID, "pr.title", pr.Title)
+			logger.InfoContext(ctx, "closing existing pull requests, no commits available", "pr.id", pr.ID, "pr.title", pr.Title)
 			err = rp.forge.ClosePullRequest(ctx, pr)
 			if err != nil {
 				return err
 			}
 		} else {
-			logger.InfoContext(ctx, "No changesets available for release")
+			logger.InfoContext(ctx, "No commits available for release")
 		}
 
 		return nil
@@ -199,8 +204,9 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error {
 		}
 	}
 
-	versionBump := VersionBumpFromChangesets(changesets)
-	nextVersion, err := SemVerNextVersion(releases, versionBump, releaseOverrides.NextVersionType)
+	versionBump := VersionBumpFromCommits(analyzedCommits)
+	// TODO: Set version in release pr
+	nextVersion, err := rp.nextVersion(releases, versionBump, releaseOverrides.NextVersionType)
 	if err != nil {
 		return err
 	}
@@ -235,7 +241,7 @@ func (rp *ReleaserPleaser) runReconcileReleasePR(ctx context.Context) error {
 		return fmt.Errorf("failed to update files with new version: %w", err)
 	}
 
-	changelogEntry, err := NewChangelogEntry(changesets, nextVersion, rp.forge.ReleaseURL(nextVersion), releaseOverrides.Prefix, releaseOverrides.Suffix)
+	changelogEntry, err := NewChangelogEntry(analyzedCommits, nextVersion, rp.forge.ReleaseURL(nextVersion), releaseOverrides.Prefix, releaseOverrides.Suffix)
 	if err != nil {
 		return fmt.Errorf("failed to build changelog entry: %w", err)
 	}

versioning.go

@@ -69,18 +69,17 @@ func SemVerNextVersion(r Releases, versionBump conventionalcommits.VersionBump,
 	return "v" + next.String(), nil
 }
 
-func VersionBumpFromChangesets(changesets []Changeset) conventionalcommits.VersionBump {
+func VersionBumpFromCommits(commits []AnalyzedCommit) conventionalcommits.VersionBump {
 	bump := conventionalcommits.UnknownVersion
 
-	for _, changeset := range changesets {
-		for _, entry := range changeset.ChangelogEntries {
+	for _, commit := range commits {
 		entryBump := conventionalcommits.UnknownVersion
 		switch {
-			case entry.BreakingChange:
+		case commit.BreakingChange:
 			entryBump = conventionalcommits.MajorVersion
-			case entry.Type == "feat":
+		case commit.Type == "feat":
 			entryBump = conventionalcommits.MinorVersion
-			case entry.Type == "fix":
+		case commit.Type == "fix":
 			entryBump = conventionalcommits.PatchVersion
 		}
 
@@ -88,7 +87,6 @@ func VersionBumpFromChangesets(changesets []Changeset) conventionalcommits.Versi
 			bump = entryBump
 		}
 	}
-	}
 
 	return bump
 }

versioning_test.go

@@ -333,86 +333,56 @@ func TestReleases_NextVersion(t *testing.T) {
 	}
 }
 
-func TestVersionBumpFromChangesets(t *testing.T) {
+func TestVersionBumpFromCommits(t *testing.T) {
 	tests := []struct {
 		name            string
-		changesets []Changeset
+		analyzedCommits []AnalyzedCommit
 		want            conventionalcommits.VersionBump
 	}{
 		{
 			name:            "no entries (unknown)",
-			changesets: []Changeset{},
+			analyzedCommits: []AnalyzedCommit{},
 			want:            conventionalcommits.UnknownVersion,
 		},
 		{
 			name:            "non-release type (unknown)",
-			changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{{Type: "docs"}}}},
+			analyzedCommits: []AnalyzedCommit{{Type: "docs"}},
 			want:            conventionalcommits.UnknownVersion,
 		},
 		{
 			name:            "single breaking (major)",
-			changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{{BreakingChange: true}}}},
+			analyzedCommits: []AnalyzedCommit{{BreakingChange: true}},
 			want:            conventionalcommits.MajorVersion,
 		},
 		{
 			name:            "single feat (minor)",
-			changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{{Type: "feat"}}}},
+			analyzedCommits: []AnalyzedCommit{{Type: "feat"}},
 			want:            conventionalcommits.MinorVersion,
 		},
 		{
 			name:            "single fix (patch)",
-			changesets: []Changeset{{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}}}},
+			analyzedCommits: []AnalyzedCommit{{Type: "fix"}},
 			want:            conventionalcommits.PatchVersion,
 		},
 		{
-			name: "multiple changesets (major)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}}},
-				{ChangelogEntries: []AnalyzedCommit{{BreakingChange: true}}},
-			},
+			name:            "multiple entries (major)",
+			analyzedCommits: []AnalyzedCommit{{Type: "fix"}, {BreakingChange: true}},
 			want:            conventionalcommits.MajorVersion,
 		},
 		{
-			name: "multiple changesets (minor)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}}},
-				{ChangelogEntries: []AnalyzedCommit{{Type: "feat"}}},
-			},
+			name:            "multiple entries (minor)",
+			analyzedCommits: []AnalyzedCommit{{Type: "fix"}, {Type: "feat"}},
 			want:            conventionalcommits.MinorVersion,
 		},
 		{
-			name: "multiple changesets (patch)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "docs"}}},
-				{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}}},
-			},
-			want: conventionalcommits.PatchVersion,
-		},
-		{
-			name: "multiple entries in one changeset (major)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}, {BreakingChange: true}}},
-			},
-			want: conventionalcommits.MajorVersion,
-		},
-		{
-			name: "multiple entries in one changeset (minor)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "fix"}, {Type: "feat"}}},
-			},
-			want: conventionalcommits.MinorVersion,
-		},
-		{
-			name: "multiple entries in one changeset (patch)",
-			changesets: []Changeset{
-				{ChangelogEntries: []AnalyzedCommit{{Type: "docs"}, {Type: "fix"}}},
-			},
+			name:            "multiple entries (patch)",
+			analyzedCommits: []AnalyzedCommit{{Type: "docs"}, {Type: "fix"}},
 			want:            conventionalcommits.PatchVersion,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equalf(t, tt.want, VersionBumpFromChangesets(tt.changesets), "VersionBumpFromChangesets(%v)", tt.changesets)
+			assert.Equalf(t, tt.want, VersionBumpFromCommits(tt.analyzedCommits), "VersionBumpFromCommits(%v)", tt.analyzedCommits)
 		})
 	}
 }