Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

"hash" key should be renamed to md5sum #13

Closed
jmtd opened this issue Mar 23, 2016 · 1 comment
Closed

"hash" key should be renamed to md5sum #13

jmtd opened this issue Mar 23, 2016 · 1 comment

Comments

@jmtd
Copy link
Contributor

jmtd commented Mar 23, 2016

The "hash" key is used to verify downloaded files. The code actually calculates an MD5 hash sum, I think the key should indicate this.

We could/should also consider using a stronger hashing algorithm, or support multiple keys (md5sum/sha256sum/etc.)

jmtd added a commit to jmtd/dogen that referenced this issue Mar 23, 2016
@jmtd
Copy link
Contributor Author

jmtd commented Jul 19, 2016

Hey, I was just revisiting this today. I still think we should do this. We have discussed this a bit off-issue and I know that several alternatives were talked about. Here's my summary

hash: c35103873c911ceb47485fb0152fb5c3

Current approach. Problem: what hash algorithm do we use here? You can't tell from the YAML

md5sum: c35103873c911ceb47485fb0152fb5c3

Suggested approach. The YAML is now self-documenting about what hash algorithm to use. Problems: should we still be using MD5? I can't remember the other objections. Was it to do with supporting multiple hashes?

hash: md5:c35103873c911ceb47485fb0152fb5c3

This was one suggestion someone made. I guess the reason was that this would not change our existing YAML "schema", either now or in the future if we added/removed support for different hash algorithms. My problem with it is that the YAML document can no longer be entirely parsed as YAML: you need to know that there are specific rules for parsing some values out of it, and those rules aren't codified in a standard.

If we wanted to retain the hash key but support multiple algorithms, I suggest

4a.

hash:
  md5: c35103873c911ceb47485fb0152fb5c3

This can also be written in flow style like

4b.

hash: {md5: c35103873c911ceb47485fb0152fb5c3}

Which is very similar to the example 3 above but is still valid YAML.

So what do you think: I'd be happy with 2 (and this PR can still be merged) but would implement 4 on request.

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

No branches or pull requests

1 participant