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.82% 97.81% -0.01%
=============================================
Files 57 57
Lines 2619 2614 -5
=============================================
- Hits 2562 2557 -5
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
| @@ -114,7 +111,7 @@ def generate_tree_from_commits( | |||
| and commit_tag not in used_tags | |||
| 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
| @@ -170,21 +167,23 @@ def process_commit_message( | |||
| changes: dict[str | None, list], | |||
| 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
| @@ -41,6 +41,9 @@ def __eq__(self, other) -> bool: | |||
| return False | |||
| return self.rev == other.rev # type: ignore | |||
|
|
|||
| def __hash__(self): | |||
commitizen/changelog.py
Outdated
| @@ -337,17 +349,17 @@ def get_oldest_and_newest_rev( | |||
| if not tags_range: | |||
| 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
2 times, most recently
from
dad215f to
1ae2d12
Compare
bearomorphism
force-pushed
the
refactor-changelog-2
branch
from
1ae2d12 to
e848508
Compare
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