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

config: extended default mime blacklist #145

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

jstucke
Copy link
Collaborator

@jstucke jstucke commented Jul 26, 2024

No description provided.

@jstucke jstucke requested review from dorpvom and maringuu July 26, 2024 08:59
@jstucke jstucke self-assigned this Jul 26, 2024
@maringuu
Copy link
Collaborator

Can you sort the list?
Also, can you make the list span over multiple lines? I find it hard to comprehend.

@jstucke
Copy link
Collaborator Author

jstucke commented Aug 19, 2024

Also, can you make the list span over multiple lines? I find it hard to comprehend.

Since the config still uses the rather restricted ini format, this is not possible (without resorting to backslashes). But it is if we convert the config to toml. I took a look at all the places where the config is used and since it looked manageable, I created a PR: #152

@maringuu
Copy link
Collaborator

Please note, that the ini format that python implements supports multiline values.
https://docs.python.org/3/library/configparser.html#supported-ini-file-structure

@jstucke
Copy link
Collaborator Author

jstucke commented Sep 4, 2024

Please note, that the ini format that python implements supports multiline values. https://docs.python.org/3/library/configparser.html#supported-ini-file-structure

Yes, technically, but do we really want that?
grafik

TOML is designed to be unambiguous and comes with native support for complex data structures. That is why we switched to it in FACT_core (also we can use pydantic for better validation)

@jstucke jstucke force-pushed the extended-default-blacklist branch from a8eb1cf to 94893d5 Compare September 4, 2024 14:37
@jstucke
Copy link
Collaborator Author

jstucke commented Sep 4, 2024

But since it does work, I changed it like you suggested. We can discuss switching to TOML in the other PR independent of the changes to the blacklist

@maringuu maringuu merged commit f7acf70 into master Sep 19, 2024
2 checks passed
@maringuu maringuu deleted the extended-default-blacklist branch September 19, 2024 09:41
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