refactor(changelog): cleanup and add test cases #1425
Conversation
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
a8be6cc to
9bb9ed9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## refactors #1425 +/- ##
=============================================
- Coverage 97.87% 97.86% -0.01%
=============================================
Files 57 57
Lines 2677 2670 -7
=============================================
- Hits 2620 2613 -7
Misses 57 57
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bearomorphism
force-pushed
the
refactor-changelog-2
branch
2 times, most recently
from
b3dac29 to
949b496
Compare
| def test_get_smart_tag_range_returns_an_empty_list_for_nonexistent_start_tag(tags): | ||
| end = tags[0] | ||
| res = changelog.get_smart_tag_range(tags, "nonexistent", end.name) | ||
| assert res[0].name == tags[1].name |
There was a problem hiding this comment.
Not sure if this is the expected behavior
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
1d03ae8 to
12166d6
Compare
commitizen/changelog.py
Outdated
| and rules.include_in_changelog(commit_tag) | ||
| ): | ||
| used_tags.append(commit_tag) | ||
| used_tags.add(commit_tag) |
There was a problem hiding this comment.
Should we add only tags that are not None?
There was a problem hiding this comment.
probably need to handle current_tag in this case as well 🤔
commitizen/changelog.py
Outdated
| change_type_map: dict[str, str] | None = None, | ||
| ): | ||
| message: dict = { | ||
| message = { |
There was a problem hiding this comment.
May I know why we're removing it?
There was a problem hiding this comment.
I'm not exactly sure when it's necessary to explicitly annotate variable types in Python, since the type is often inferred from the assigned value.
There was a problem hiding this comment.
I just added dict[str, str | list[str] | Any] here
commitizen/git.py
Outdated
| return False | ||
| return self.rev == other.rev # type: ignore | ||
|
|
||
| def __hash__(self): |
commitizen/changelog.py
Outdated
| raise NoCommitsFoundError("Could not find a valid revision range.") | ||
|
|
||
| oldest_rev: str | None = tags_range[-1].name | ||
| oldest_rev = tags_range[-1].name |
There was a problem hiding this comment.
May I know why we want to remove the type here?
There was a problem hiding this comment.
The type of oldest_rev here is actually str, so I removed it.
But I just pushed another refactor to make the type annotation reasonable.
commitizen/changelog.py
Outdated
| return [] | ||
|
|
||
| if not has_newest: | ||
| return tags[1:] |
There was a problem hiding this comment.
I'm a bit confused. Why is the 1 here? I must admit that I'm not that familiar with this part 🤦♂️
There was a problem hiding this comment.
I remember I was trying to align the behavior before and after the refactoring
There was a problem hiding this comment.
We can hold off this part and separate it in another PR though. The algorithm here is a bit confusing.
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
12166d6 to
5440afd
Compare
bearomorphism
force-pushed
the
refactor-changelog-2
branch
3 times, most recently
from
1ae2d12 to
e848508
Compare
Lee-W
force-pushed
the
refactors
branch
2 times, most recently
from
baee83e to
a9cd957
Compare
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
e848508 to
2f05414
Compare
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
2f05414 to
0fb8ce7
Compare
2 participants
Description
Checklist
Code Changes
poetry alllocally to ensure this change passes linter check and testsDocumentation Changes
poetry doclocally to ensure the documentation pages renders correctlyExpected Behavior
Steps to Test This Pull Request
Additional Context