Skip to content
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

Optional fix for quotation marks #307

Merged
merged 7 commits into from
Nov 10, 2024
Merged

Optional fix for quotation marks #307

merged 7 commits into from
Nov 10, 2024

Conversation

bohning
Copy link
Owner

@bohning bohning commented Nov 3, 2024

No description provided.

src/usdb_syncer/constants.py Show resolved Hide resolved
src/usdb_syncer/song_txt/auxiliaries.py Outdated Show resolved Hide resolved
src/usdb_syncer/song_txt/auxiliaries.py Outdated Show resolved Hide resolved
src/usdb_syncer/constants.py Show resolved Hide resolved
src/usdb_syncer/constants.py Outdated Show resolved Hide resolved

marks_total = marks_total + 1

return value, marks_total, marks_fixed
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear what this function returns. A dataclass or named tuple would be nice.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Like this?

from typing import NamedTuple

# Define the named tuple class
class ReplacementResult(NamedTuple):
    value: str
    marks_total: int
    marks_fixed: int

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that's one way.

@RumovZ
Copy link
Collaborator

RumovZ commented Nov 10, 2024

As the code is becoming more complicated, it's hard to tell if it does the right thing just from looking at it. Some unit tests would be helpful.

@bohning bohning merged commit ff194c8 into main Nov 10, 2024
1 check passed
@bohning bohning deleted the fix_quotation_marks branch November 10, 2024 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants