Add an option -since-latest-release #2
Conversation
buchanae
left a comment
There was a problem hiding this comment.
This is great, thanks for doing this. I've been wanting this feature.
I have a few minor comments, but I have tried this and it seems to work.
ghrn/ghrn.go
Outdated
| StopAt int | ||
| // IncludeCommits will include commmits messages for each PR. | ||
| IncludeCommits bool | ||
| // StopAtLatestRelease will stop at latest release commit. |
There was a problem hiding this comment.
Let's call this "SinceLatestRelease" to match the flag. Docs would be "SinceLatestRelease will only include PRs and commits merged since the latest release tag."
ghrn/ghrn.go
Outdated
| comp, _, err := cl.Repositories.CompareCommits(ctx, conf.Org, conf.Repo, rls.GetTagName(), repo.GetDefaultBranch()) | ||
| if err != nil { | ||
| return fmt.Errorf("compare commitse: %s..%s %+v", rls.GetTagName(), repo.GetDefaultBranch(), err) | ||
| } |
There was a problem hiding this comment.
I think these are only needed for when conf.SinceLatestRelease is true. Can you move these to a helper, and only use them if we're conf.SinceLatestRelease == true? This would avoid a few calls to github and maybe save 1-2 seconds in some cases.
ghrn/ghrn.go
Outdated
| var newCommits []string | ||
| for _, commit := range comp.Commits { | ||
| newCommits = append(newCommits, commit.GetCommit().GetTree().GetSHA()) | ||
| } |
There was a problem hiding this comment.
Can you put this in a helper?
Perhaps something like:
function commitHashes(commits []*github.RepositoryCommit) []string {}
newCommits := commitHashes(comp.Commits)
ghrn/ghrn.go
Outdated
| } | ||
|
|
||
| if conf.StopAtLatestRelease && !any(prCommits, newCommits) { | ||
| // stop any new commits do not contains pr commits |
There was a problem hiding this comment.
A little help with the English: "Stop when a PR doesn't contain any commits from since the latest release."
There was a problem hiding this comment.
Also, technically it's possible to have a PR that was merged to a branch besides master after the latest release, in which case this code would stop early and produce incorrect release notes.
readme.md
Outdated
| - PR #514 build: fix release notes command | ||
| ``` | ||
|
|
||
| You can generating notes with PR that not included latest release: |
There was a problem hiding this comment.
"You can generate notes for only PRs merged since the latest release:"
|
Thank you for the merge and your carefully review!! And sorry for my poor English... I just added commit duck8823@beebe15 to respond to the review comment #2 (comment) after your merge... |
|
@duck8823 can you open a new PR with the commit I missed please? |
Hi, thank you for great tool !
But I wont a option that notes only new PRs.
Here, I just add a
-since-latest-release.This option analyze with latest release tag, compared commit sha and pr's commits from GitHub, and stop notes already released PR automatically.