-
-
Notifications
You must be signed in to change notification settings - Fork 302
refactor(BaseFormat): merge ChangelogFormat into BaseFormat #1612
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
base: master
Are you sure you want to change the base?
refactor(BaseFormat): merge ChangelogFormat into BaseFormat #1612
Conversation
|
|
||
| def parse_version_from_title(self, line: str) -> VersionTag | None: | ||
| raise NotImplementedError("parse_version_from_title is not implemented") | ||
|
|
||
| def parse_title_level(self, line: str) -> int | None: | ||
| raise NotImplementedError("parse_title_level is not implemented") |
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.
Not sure if we're going to implement these.
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.
I think not. Those methods look like protected and is only used in get_metadata_from_file but this class overrides the function get_metadata_from_file
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4-9-2 #1612 +/- ##
=========================================
Coverage ? 98.29%
=========================================
Files ? 58
Lines ? 2695
Branches ? 0
=========================================
Hits ? 2649
Misses ? 46
Partials ? 0
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:
|
|
I think @noirbizarre might be a better person to take a look at this one. My guts feeling tells me |
Description
BaseFormatalready looks like a base class for changelog format, but it has a parent class protocolChangelogFormat. It is confusing to me.Additionally, the protocol
ChangelogFormatdeclares implementations of methods. I consider it confusing too.This change addresses the unnecessary complexity and remove
ChangelogFormatcompletely from the codebase. The alternative now isBaseFormat.I am not sure if this is a breaking change. Please help to review.