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

hash/hashmap: replace MakeSeed with NewSeed? #35060

Closed
mdempsky opened this issue Oct 21, 2019 · 2 comments
Closed

hash/hashmap: replace MakeSeed with NewSeed? #35060

mdempsky opened this issue Oct 21, 2019 · 2 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Milestone

Comments

@mdempsky
Copy link
Contributor

mdempsky commented Oct 21, 2019

In #28322, @rsc said "it sounds like we have converged on: [...] func NewSeed() Seed".

@randall77 later proposed "It might be nice to be able to serialize/deserialize a Seed. Probably not necessary for v1." "...or construct a Seed from an int64."

@rsc subsequently stated "I agree about @randall77's clarifications and added them above.", incorporating the other changes @randall77 proposed in his comments (i.e., Add and Hash semantics), but not these Seed changes (which weren't addressed).

However, CL 186877 did implement MakeSeed(uint64) Seed rather than NewSeed() Seed,

Filing as a release blocker just to make sure we're agreed on the API that we want exposed here before we're committed to supporting it.

--

My 2c: Since seeds only have meaning within a single process, it seems just as easy to simply call hash.NewSeed and then pass the hash.Seed value around as it is to generate a random uint64, pass that around, and call hash.MakeSeed.

@mdempsky mdempsky added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker labels Oct 21, 2019
@mdempsky mdempsky added this to the Go1.14 milestone Oct 21, 2019
@rsc rsc changed the title bytes/hash: replace MakeSeed with NewSeed? hash/hashmap: replace MakeSeed with NewSeed? Nov 4, 2019
@rsc
Copy link
Contributor

rsc commented Nov 4, 2019

CL 205069 does this (removes the argument).

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/205069 mentions this issue: hash/maphash: revise API to be more idiomatic

@golang golang locked and limited conversation to collaborators Nov 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Projects
None yet
Development

No branches or pull requests

3 participants