-
Notifications
You must be signed in to change notification settings - Fork 173
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
feat: add support for hash checking #133
Conversation
Not necessarily, you can include the algorithm in the build metadata as well.
|
Ah yes, that’s right – IIUC dot is |
A dot is perfectly valid, see the examples in https://regex101.com/r/vkijKf/1/ from https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite elegant solution 👍
I just realised I'm checking the hash of the JSON metadata instead of the tarball 🤦♂️ Converting to draft for now. |
tests/main.test.ts
Outdated
const testedPackageManagers: Array<[string, string]> = [ | ||
[`yarn`, `1.22.4`], | ||
[`yarn`, `1.22.4+md5.faf483d50aa8ccbdc802efa0cac5d4d3`], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we don't test sha1
, which is what #137 uses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I've switched to SHA1 and SHA224 in the tests as those are more credible options for hash algorithms.
I'm not sure it's how we want to do it, according to semver.org, we could use build suffix to the version number (e.g.
[email protected]+hash
),that would force us to pick a hashing algorithm (which is not a problem in itself, but could create issues in the future if it becomes outdated).EDIT: with a new version we're now using semver compatible suffix for the hash.
Fixes: #37