Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always include end of commit range #107

Merged
merged 3 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ unit: $(RESULTS_DIR) fixtures ## Run unit tests (with coverage)
fixtures:
$(call title,Generating test fixtures)
cd internal/git/test-fixtures && make
cd chronicle/release/releasers/github/test-fixtures && make
wagoodman marked this conversation as resolved.
Show resolved Hide resolved

fixtures-fingerprint:
find internal/git/test-fixtures/*.sh -type f -exec md5sum {} + | awk '{print $1}' | sort | md5sum | tee internal/git/test-fixtures/cache.fingerprint && echo "$(FIXTURE_CACHE_BUSTER)" >> internal/git/test-fixtures/cache.fingerprint
Expand All @@ -182,7 +183,7 @@ $(SNAPSHOT_DIR): ## Build snapshot release binaries and packages
$(TEMP_DIR)/goreleaser build --snapshot --skip-validate --rm-dist --config $(TEMP_DIR)/goreleaser.yaml

.PHONY: changelog
changelog: clean-changelog CHANGELOG.md
changelog: clean-changelog $(CHANGELOG)
@docker run -it --rm \
-v $(shell pwd)/CHANGELOG.md:/CHANGELOG.md \
rawkode/mdv \
Expand Down
2 changes: 2 additions & 0 deletions chronicle/release/releasers/github/gh_release.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"golang.org/x/oauth2"
)

type releaseFetcher func(user, repo, tag string) (*ghRelease, error)

type ghRelease struct {
Tag string
Date time.Time
Expand Down
161 changes: 112 additions & 49 deletions chronicle/release/releasers/github/summarizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,26 @@ type Config struct {
}

type Summarizer struct {
git git.Interface
userName string
repoName string
config Config
git git.Interface
userName string
repoName string
config Config
releaseFetcher releaseFetcher
}

// changeScope is used to describe the start and end of a changes made in a repo.
type changeScope struct {
Commits []string
Start changePoint
End changePoint
}

// changePoint is a single point on the timeline of changes in a repo.
type changePoint struct {
Ref string
Tag *git.Tag
Inclusive bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make sure I understand here. Inclusive means "the changelog should include this point"? And then below, when we generate a changelog that includes the first commit in the repo, we need to emit an inclusive change point (because the first commit is part of the first release), but subsequent releases, where we're asking for something like "commits since the tag v0.5.1", we should make an exclusive range. Is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's 100% correct 👍

Timestamp *time.Time
}

func NewSummarizer(gitter git.Interface, config Config) (*Summarizer, error) {
Expand All @@ -56,15 +72,16 @@ func NewSummarizer(gitter git.Interface, config Config) (*Summarizer, error) {
log.WithFields("owner", user, "repo", repo).Debug("github summarizer")

return &Summarizer{
git: gitter,
userName: user,
repoName: repo,
config: config,
git: gitter,
userName: user,
repoName: repo,
config: config,
releaseFetcher: fetchRelease,
}, nil
}

func (s *Summarizer) Release(ref string) (*release.Release, error) {
targetRelease, err := fetchRelease(s.userName, s.repoName, ref)
targetRelease, err := s.releaseFetcher(s.userName, s.repoName, ref)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -100,78 +117,124 @@ func (s *Summarizer) LastRelease() (*release.Release, error) {
return nil, fmt.Errorf("unable to find latest release")
}

//nolint:funlen
func (s *Summarizer) Changes(sinceRef, untilRef string) ([]change.Change, error) {
var changes []change.Change
var err error

var includeStart, includeEnd bool
scope, err := s.getChangeScope(sinceRef, untilRef)
if err != nil {
return nil, err
}

var sinceTag *git.Tag
sinceHash := sinceRef
if sinceRef != "" {
sinceTag, err = s.git.SearchForTag(sinceRef)
if err != nil {
return nil, err
}
includeStart = false
} else {
includeStart = true
if scope == nil {
return nil, fmt.Errorf("unable to find start and end of changes: %w", err)
}

var sinceTime *time.Time
if sinceTag != nil {
sinceRelease, err := fetchRelease(s.userName, s.repoName, sinceTag.Name)
if err != nil {
return nil, fmt.Errorf("unable to fetch release %q: %w", sinceTag.Name, err)
}
sinceTime = &sinceRelease.Date
return s.changes(*scope)
}

func (s *Summarizer) getChangeScope(sinceRef, untilRef string) (*changeScope, error) {
sinceTag, sinceRef, includeStart, sinceTime, err := s.getSince(sinceRef)
if err != nil {
return nil, err
}

var untilTag *git.Tag
untilHash := untilRef
var untilTime *time.Time
if untilRef != "" {
untilTag, err = s.git.SearchForTag(untilRef)
if err != nil {
return nil, err
}
includeEnd = false
untilTime = &untilTag.Timestamp
} else {
untilHash, err = s.git.HeadTagOrCommit()
untilRef, err = s.git.HeadTagOrCommit()
if err != nil {
return nil, err
}
includeEnd = true
}

var includeCommits []string
if s.config.ConsiderPRMergeCommits {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what this config does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this option never was added to the documentation, I can update that in a separate PR (here's the PR that put this feature in #40 ) . In short, when enabled the commits that comprise the release can affect which PRs get included into the release notes.

includeCommits, err = s.git.CommitsBetween(git.Range{
SinceRef: sinceHash,
UntilRef: untilHash,
SinceRef: sinceRef,
UntilRef: untilRef,
IncludeStart: includeStart,
IncludeEnd: includeEnd,
IncludeEnd: true,
})
if err != nil {
return nil, fmt.Errorf("unable to fetch commit range: %v", err)
}
}

return &changeScope{
Commits: includeCommits,
Start: changePoint{
Ref: sinceRef,
Tag: sinceTag,
Inclusive: includeStart,
Timestamp: sinceTime,
},
End: changePoint{
Ref: untilRef,
Tag: untilTag,
Inclusive: true,
Timestamp: untilTime,
},
}, nil
}

func (s *Summarizer) getSince(sinceRef string) (*git.Tag, string, bool, *time.Time, error) {
var err error
var sinceTag *git.Tag
var includeStart bool

if sinceRef != "" {
sinceTag, err = s.git.SearchForTag(sinceRef)
if err != nil {
return nil, "", false, nil, err
}
}

var sinceTime *time.Time
if sinceTag != nil {
sinceRelease, err := s.releaseFetcher(s.userName, s.repoName, sinceTag.Name)
if err != nil {
return nil, "", false, nil, fmt.Errorf("unable to fetch release %q: %w", sinceTag.Name, err)
}
if sinceRelease != nil {
sinceTime = &sinceRelease.Date
}
}

if sinceTag == nil {
sinceRef, err = s.git.FirstCommit()
if err != nil {
return nil, "", false, nil, fmt.Errorf("unable to find first commit: %w", err)
}
includeStart = true
}

return sinceTag, sinceRef, includeStart, sinceTime, nil
}

log.Debugf("release comprised of %d commits", len(includeCommits))
logCommits(includeCommits)
func (s *Summarizer) changes(scope changeScope) ([]change.Change, error) {
var changes []change.Change

if s.config.ConsiderPRMergeCommits {
log.Debugf("release comprises %d commits", len(scope.Commits))
logCommits(scope.Commits)
}

allMergedPRs, err := fetchMergedPRs(s.userName, s.repoName, sinceTime)
allMergedPRs, err := fetchMergedPRs(s.userName, s.repoName, scope.Start.Timestamp)
if err != nil {
return nil, err
}

log.WithFields("since", sinceTime).Debugf("total merged PRs discovered: %d", len(allMergedPRs))
log.WithFields("since", scope.Start.Timestamp).Debugf("total merged PRs discovered: %d", len(allMergedPRs))

if s.config.IncludePRs {
changes = append(changes, changesFromStandardPRFilters(s.config, allMergedPRs, sinceTag, untilTag, includeCommits)...)
changes = append(changes, changesFromStandardPRFilters(s.config, allMergedPRs, scope.Start.Tag, scope.End.Tag, scope.Commits)...)
}

allClosedIssues, err := fetchClosedIssues(s.userName, s.repoName, sinceTime)
allClosedIssues, err := fetchClosedIssues(s.userName, s.repoName, scope.Start.Timestamp)
if err != nil {
return nil, err
}
Expand All @@ -180,22 +243,22 @@ func (s *Summarizer) Changes(sinceRef, untilRef string) ([]change.Change, error)
allClosedIssues = filterIssues(allClosedIssues, excludeIssuesNotPlanned(allMergedPRs))
}

log.WithFields("since", sinceTime).Debugf("total closed issues discovered: %d", len(allClosedIssues))
log.WithFields("since", scope.Start.Timestamp).Debugf("total closed issues discovered: %d", len(allClosedIssues))

if s.config.IncludeIssues {
if s.config.IssuesRequireLinkedPR {
changes = append(changes, changesFromIssuesLinkedToPrs(s.config, allMergedPRs, sinceTag, untilTag, includeCommits)...)
changes = append(changes, changesFromIssuesLinkedToPrs(s.config, allMergedPRs, scope.Start.Tag, scope.End.Tag, scope.Commits)...)
} else {
changes = append(changes, changesFromIssues(s.config, allMergedPRs, allClosedIssues, sinceTag, untilTag)...)
changes = append(changes, changesFromIssues(s.config, allMergedPRs, allClosedIssues, scope.Start.Tag, scope.End.Tag)...)
}
}

if s.config.IncludeUnlabeledIssues {
changes = append(changes, changesFromUnlabeledIssues(s.config, allMergedPRs, allClosedIssues, sinceTag, untilTag)...)
changes = append(changes, changesFromUnlabeledIssues(s.config, allMergedPRs, allClosedIssues, scope.Start.Tag, scope.End.Tag)...)
}

if s.config.IncludeUnlabeledPRs {
changes = append(changes, changesFromUnlabeledPRs(s.config, allMergedPRs, sinceTag, untilTag, includeCommits)...)
changes = append(changes, changesFromUnlabeledPRs(s.config, allMergedPRs, scope.Start.Tag, scope.End.Tag, scope.Commits)...)
}

return changes, nil
Expand Down
Loading