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 some caveats about changing the default hash algorithm #178

Merged
merged 6 commits into from
Oct 4, 2023

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Sep 30, 2023

and advice on keeping the algorithm used along with any results.

(Note, when this PR is created, spec-prod is having some issues resolving aspects of the document, so the PR Preview link might show errors. This GitHack version may provide better results).

Fixes #176.


Preview | Diff

… on keeping the algorithm used along with any results.

Fixes #176.
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few suggestions. Thanks!

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
Copy link
Contributor

@yamdan yamdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @gkellogg! I just added a few comments.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved
Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (with one minor nit).

Copy link
Contributor

@peacekeeper peacekeeper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @msporny 's suggestion about the internal hash algorithm.

spec/index.html Outdated Show resolved Hide resolved
gkellogg and others added 2 commits October 3, 2023 22:37
Co-authored-by: Manu Sporny <[email protected]>
Co-authored-by: Dan Yamamoto <[email protected]>
Co-authored-by: David I. Lehn <[email protected]>
Copy link
Contributor

@yamdan yamdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@gkellogg gkellogg merged commit 7e08a16 into main Oct 4, 2023
1 check passed
@gkellogg gkellogg deleted the hash-algo-clarifications branch October 4, 2023 14:57
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.

Issues with parameterized hashing algorithms used internally
7 participants