FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

refactor(ChangelogFormat): refactor get_metadata_from_file for better readability and type by bearomorphism · Pull Request #1596 · commitizen-tools/commitizen · GitHub

/ commitizen Public

refactor(ChangelogFormat): refactor get_metadata_from_file for better readability and type - #1596

Draft
bearomorphism wants to merge 1 commit into
commitizen-tools:masterfrom
bearomorphism:refactor-getmetadata
ConversationCommits (1)ChecksFiles changed
Draft

refactor(ChangelogFormat): refactor get_metadata_from_file for better readability and type#1596
bearomorphism wants to merge 1 commit into
commitizen-tools:masterfrom
bearomorphism:refactor-getmetadata

Conversation

bearomorphism commented Sep 12, 2025

Copy link
Copy Markdown
Collaborator

… readability and better type

Description

Checklist

Code Changes

  • Add test cases to all the changes you introduce
  • Run poetry all locally to ensure this change passes linter check and tests
  • Manually test the changes:
    • Verify the feature/bug fix works as expected in real-world scenarios
    • Test edge cases and error conditions
    • Ensure backward compatibility is maintained
    • Document any manual testing steps performed
  • Update the documentation for the changes

Documentation Changes

  • Run poetry doc locally to ensure the documentation pages renders correctly
  • Check and fix any broken links (internal or external) in the documentation

When running poetry doc, any broken internal documentation links will be reported in the console output like this:

INFO    -  Doc file 'config.md' contains a link 'commands/bump.md#-post_bump_hooks', but the doc 'commands/bump.md' does not contain an anchor '#-post_bump_hooks'.

Expected Behavior

Steps to Test This Pull Request

Additional Context

bearomorphism commented Sep 12, 2025
View reviewed changes
Comment thread commitizen/changelog_formats/base.py
unreleased = self.parse_title_level(line)
lines = file.readlines()
for index, line in enumerate(line.strip().lower() for line in lines):
parsed_unreleased_level = self.parse_title_level(line)

bearomorphism Sep 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we can assume that self.parse_title_level is always a pure function? Then we don't have to call it twice here.

Lee-W Nov 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If we want to make this assumption, we'll need to make this a static method instead, which makes sense but could be seen as a breaking change.

Lee-W Nov 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

but making that a static method is a good idea

bearomorphism commented Sep 12, 2025
View reviewed changes
Comment thread commitizen/changelog_formats/base.py
out_metadata.latest_version_position = index
break # there's no need for more info
if meta.unreleased_start is not None and meta.unreleased_end is None:
meta.unreleased_end = index

bearomorphism Sep 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This index is unbounded if the file is empty, although this never happens for all code paths

codecov Bot commented Sep 12, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.25%. Comparing base (120d514) to head (edf22a8).
⚠️ Report is 808 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1596      +/-   ##
==========================================
+ Coverage   97.33%   98.25%   +0.91%     
==========================================
  Files          42       58      +16     
  Lines        2104     2692     +588     
==========================================
+ Hits         2048     2645     +597     
+ Misses         56       47       -9
Flag Coverage Δ
unittests 98.25% <100.00%> (+0.91%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Lee-W added this to the 4.9.2 milestone Sep 12, 2025
bearomorphism mentioned this pull request Sep 15, 2025
Merged
Lee-W changed the title refactor(ChangelogFormat): refactor get_metadata_from_file for better… refactor(ChangelogFormat): refactor get_metadata_from_file for better readability and type Nov 8, 2025
Lee-W reviewed Nov 8, 2025
View reviewed changes
Comment thread commitizen/changelog_formats/base.py
unreleased = self.parse_title_level(line)
lines = file.readlines()
for index, line in enumerate(line.strip().lower() for line in lines):
parsed_unreleased_level = self.parse_title_level(line)

Lee-W Nov 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If we want to make this assumption, we'll need to make this a static method instead, which makes sense but could be seen as a breaking change.

Comment thread commitizen/changelog_formats/base.py
unreleased: int | None = None
if "unreleased" in line:
unreleased = self.parse_title_level(line)
lines = [line.strip().lower() for line in file.readlines()]

Lee-W Nov 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

loop through it twice, probably not worth it?

Lee-W marked this pull request as draft November 8, 2025 10:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Lee-W Lee-W left review comments
woile Awaiting requested review from woile
noirbizarre Awaiting requested review from noirbizarre

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

pr-status: wait-for-response type: refactor

Projects

None yet

Milestone

4.10.1

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL