[nextMajor] remove metadata conversion#190
Conversation
rgaudin
left a comment
There was a problem hiding this comment.
@aryanA101a, thank you ; but:
- we still need to encode to bytes.
- if you remove code, you should remove associated test. This proves you didn't run the tests locally… well you should
FYI we'll only merge this once we want to release a new major version as this is an API break and we're in no rush to do so
|
@rgaudin Sorry, my bad. |
|
@aryanA101a Any update? |
rgaudin
left a comment
There was a problem hiding this comment.
LGTM ;
Must NOT be merged!. This is an API break; we'll merge once we know we're ready to publish a major version
|
converted to draft to prevent accidental merge |
|
I don't think this is sufficient: type hints should be simplified as well to explicitly mention these methods support only bytes and str values now (i.e. conversion must be done by python-scraperlib or any other "user") |
|
Thank you @benoit74 ; that's right and I forgot @aryanA101a to mention that you should not force-push after having asked for a review ; it makes (and made in that case) reviewing a real pain. |
|
@aryanA101a please update type hints as suggested |
Fixes #188