Closed kevina closed 6 years ago
@Stebalien do you think we could get this change in ASAP? Right now it doesn't look like these functions are used. Once they become used this change will become more difficult.
So, aren't we planning on replacing these functions with a Format
struct and a NewFormat
function?
That is, I'd rather deprecate and remove these functions and move to that than to break things.
Possible way forward:
Basically, changing the signature will be the same amount of work as deprecating and removing.
@Stebalien
Not merging this P.R. is partly blocking #50.
A grep of the entire source tree found that these functions are used in only one place outside of go-cid in go-libp2p-routing-helpers
in parallel_test.go
.
My plan was to get #40/#50 in before #52.
As far work, all that needs to be done is to just merge this P.R. We don't need to GX publish just yet. If this gets GX published before #52 gets in fixing the signature in the one place is not a lot of work compared to the updating all the places that use go-cid
.
As far work, all that needs to be done is to just merge this P.R. We don't need to GX publish just yet. If this gets GX published before #52 gets in fixing the signature in the one place is not a lot of work compared to the updating all the places that use go-cid.
That'll still break anyone calling go test ./...
. While we use gx, we try not to rely on it.
Also, there are other users:
If an interface is broken and we'll want to break it eventually, I'm usually all for breaking it sooner rather than later. However, given that we want to simply replace these functions, I'd rather leave them as-is (they still work, even if they allow invalid states) and then replace and deprecate them.
Closing in favor of #53.
I don't think these two functions are really used yet, so this shouldn't break anything.
NewPrefixV0
doesn't take any parameters, since there is only one possible legal combination.NewCidV1
is expanded to also take the multihash length. Even though the length 99% of the time will be-1
it still makes sense to require it. This gives the function the opportunity to convert the length value of-1
to the default length. The callee of the function is already likely to have the length value and needs to do something with it. With out being able to pass it in the most logical think to do would be to set it afterwards, for example:which doesn't allow NewPrefix to set the value to the default length.
Closes #49.