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

Add script to simplify adding a missing type. #91

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dosas
Copy link

@dosas dosas commented Dec 27, 2020

Should simplify: #45

@h2non
Copy link
Owner

h2non commented Jan 4, 2021

I'm not very convinced about this code generation approach.

It will lead to inconsistency in the way to declare matches in the library unless all matches are ported to be defined the same way consistently within the package, which is not something I'm very inclined to. Typically, dynamic code generation has its advantages where relevant boilerplate steps are involved, but I don't see much value here. I would rather prefer the explicitness of persisted and evaluable code without any code generation process involved.

@dosas
Copy link
Author

dosas commented Jan 5, 2021

I get your point about the inconsistencies. You mean the comparison by bytearray versus the or, and clauses?

I disagree on the boilerplate because in order to add 4 bytes, one extension, and a mime type I have to write about 8 lines of code (that only differs in the mentioned properties), add the type to the README, and to __init__.

I don't get this point:

I would rather prefer the explicitness of persisted and evaluable code without any code generation process involved.

The generated code is committed (I did all my PRs using this generation appraoch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants