-
-
Notifications
You must be signed in to change notification settings - Fork 201
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
fix(changelog): include the root commit when --latest
is used with one tag
#901
Conversation
Thanks for opening this pull request! Please check out our contributing guidelines! ⛰️ |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #901 +/- ##
==========================================
- Coverage 40.10% 40.06% -0.03%
==========================================
Files 21 21
Lines 1671 1675 +4
==========================================
+ Hits 670 671 +1
- Misses 1001 1004 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! The implementation could be simplified a bit :)
Use the fact that a range contains (or doesn't) contain ".." as a discriminant between the two cases: - ".." means full (left-exclusive) range between two commits; - no ".." means everything from the root commit (inclusive) to the commit sha in the range
eda2fd9
to
06f96c2
Compare
btw can you revert fa85e95 those are not in the scope of this PR |
Hi, I think the |
Sure, np |
e625a81
to
90e0202
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
--latest
with one tag should include root commit--latest
is used with one tag
Congrats on merging your first pull request! ⛰️ |
Description, Motivation and Context
This is a proof of concept implementation for the fix of including the
root
commit of a repository whengit-cliff
is called with the--latest
flag and only ONE tag exists in the repository.Ranges passed to to
Repository::commits
are left-exclusive. This works well when we have two or more tags:We'll get the range argument
sha0..sha3
, which will make the::commits
call capture only thesha1
,sha2
andsha3
commits, which is both expected and great.However, when dealing with a first release, and a single tag, we'd still like the root commit of the repository to be included in the report. Think of it as having a tag before the root commit itself.
In this case, with the current implementation of
::commits
we get the rangeroot_sha..single_tag_sha
which, being left-exclusive, will exclude the root commit from the report.My implementation addresses this by adding a flag to
::commits
to tell it to include the root commit explicitly. When this boolean flag is present, we chop the range and include everything up to the..single_tag_sha
.How Has This Been Tested?
Added both a unit test and adapted the
test-latest-with-one-tag
fixture for the integration tests.Types of Changes
Checklist: