chelnak / gh-changelog

A gh cli extension that will make your changelogs ✨
MIT License
109 stars 4 forks source link

Add --ancestors-only and --filter flags + some much needed maintenance #156

Open chelnak opened 3 months ago

chelnak commented 3 months ago

This change adds two new flags to the new command.

--ancestors-only

When this flag is passed, the builder will filter out any tags that are not an ancestor of the current branch. This is a possible fix for #145 .

--filter

When this flag is passed, the builder will filter out any tags that do not match the given RE2 regexp.

Additionally this PR contains some needed fixes and 💅

chelnak commented 3 months ago

Hey @h0tw1r3

It's a pretty meaty PR but it would be awesome if you could pull it down and take it for a spin.

Your comment in #145 about the tags api response is right! The PR actually switches to using Git to get the tags. Then uses merge-base to determine if the commit is an ancestor of the current branch or not.

I think the original decision to go with the api for tags was that when run in CI, the results were not consistent with some local tests. IIRC, I mainly tested on some big module repos with big complicated histories (puppetlabs-chocolatey & puppetlabs-docker), so i'd be interested to see how this performs.

With regards to the suggestion you made in #145, if we can get all of the information needed with one git command.. that would be pretty sweet.

chelnak commented 2 months ago

Looks like i've missed something in the tests.. will investigate and update tomorrow

h0tw1r3 commented 2 months ago

Hey @h0tw1r3

It's a pretty meaty PR but it would be awesome if you could pull it down and take it for a spin.

Yeah, lol. I'd say. Code review won't be a thing, but I'll test it on a few repos.

Your comment in #145 about the tags api response is right! The PR actually switches to using Git to get the tags. Then uses merge-base to determine if the commit is an ancestor of the current branch or not.

I think the original decision to go with the api for tags was that when run in CI, the results were not consistent with some local tests. IIRC, I mainly tested on some big module repos with big complicated histories (puppetlabs-chocolatey & puppetlabs-docker), so i'd be interested to see how this performs.

The inconsistency usually comes from how the repo was cloned, reach-ability (orphans with tags, oh my), and mixing tag types. The GH API consistently returns everything.

With regards to the suggestion you made in #145, if we can get all of the information needed with one git command.. that would be pretty sweet.

chelnak commented 2 months ago

🤣 I will probably split this up to be honest. It would make better sense as smaller PRs.

In the meantime, the concept is here and it's good for testing.

h0tw1r3 commented 2 months ago

Filter doesn't work for me, or I'm using it wrong. --filter="[0-7]*" on the puppet repo filters nothing.

Calling merge-base for every tag is very inefficient, almost twice as long to execute on a large repo with hundreds of tags. Did a test with it removed, by optionally adding --merged=HEAD to Git.Tags() exec. Twice as fast without exec bomb.

Before

time ~/dev/gh-changelog/gh-changelog new --ancestors-only
✓ Open CHANGELOG.md or run 'gh changelog show' to view your changelog.

real    1m55.764s
user    0m41.955s
sys     0m2.730s

After

time ~/dev/gh-changelog/gh-changelog new --ancestors-only
✓ Open CHANGELOG.md or run 'gh changelog show' to view your changelog.

real    1m15.187s
user    0m0.576s
sys     0m0.156s

Confirmed CHANGELOG.md is the same before and after.

https://github.com/chelnak/gh-changelog/compare/gh-changelog-145...h0tw1r3:gh-changelog:gh-changelog-145-faster