| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
please no pathlib, dataclasses, or backslashes -- really you probably don't need any classes at all. I'm also not sure why the strange sha256 -- use hexdigest which is way more standardized. there's a lot of code here and almost zero tests so it's not in a reviewable state -- please convert your PR to draft until you're ready for a first round |
Sorry, something went wrong.
|
What do you mean by
The " the strange sha256" is subresource integrity. What do you mean by "no backslashes"? |
Sorry, something went wrong.
your code has a bunch of classes with nontrivial methods -- simple plain-old-data classes is preferred -- check the approaches used by most of the rest of the codebase
just because it is standardized doesn't mean it is good -- we aren't optimizing for http headers where characters are at a minimum so use normal hexdigest which is far more common and much easier to compute
your source code contains many very very ugly backslashes |
Sorry, something went wrong.
This commit implements pre-commit#1453 (comment)
@asottile, normal checksum, i.e. the output from sha256sum or md5sum are not in hex. If we use hexdigest here, then you also need something to generate those hexdigest, right? |
Sorry, something went wrong.
what do you mean yes they are |
Sorry, something went wrong.
|
Ah never mind, for some reason I thought md5sum and sha256sum outputs string in base64 format. |
Sorry, something went wrong.
|
I recently build something similar that has a format I was pretty happy with -- might be able to use that format or similar here: https://github.com/asottile/setuptools-download |
Sorry, something went wrong.
|
@asottile , do you have a preferred way of handling the different OS and architecture? |
Sorry, something went wrong.
|
setuptools-download handles that with markers -- essentially filters on sys.platform, platform.machine() and os.name (though os.name is perhaps redundant) |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This commit implements #1453 (comment)