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

Fixed incorrect extension management #30

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Conversation

lgiordani
Copy link
Collaborator

@justinmayer I'm sorry to report that the solution you suggested here does not work. I had to restore the original format of the variable (a list) and add a weird typing to satisfy Ruff.

@justinmayer
Copy link
Contributor

Thank you for letting me know about this. I think see what happened here.

I originally intended to change the assignment to a non-mutable tuple:

file_extensions = ("mau")

… forgetting for a moment that a single-item tuple must have a trailing comma — otherwise it just becomes a string. So when I later ran Black, it dropped the "unnecessary" parentheses, and I didn't notice the change.

In short, unless there is a reason for this value to be mutable, I think perhaps my originally-intended change might be the best solution:

file_extensions = ("mau",)

Would that work?

(Side note: astral-sh/ruff#5243)

@lgiordani
Copy link
Collaborator Author

That makes sense @justinmayer, I didn't think about it, even though it tripped me over multiple times (usually converting values with a comma into tuples, not the other way around). Anyway, have a look if that is OK now. Thanks!

Copy link
Contributor

@justinmayer justinmayer left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this!

@lgiordani lgiordani merged commit 3d32e21 into main Sep 11, 2023
12 checks passed
@lgiordani lgiordani deleted the fix-wrong-extension-management branch September 11, 2023 06:38
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