-
Notifications
You must be signed in to change notification settings - Fork 47
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
default cidv1 to base32 #85
Conversation
f05385d
to
f04f921
Compare
We'd like to make go-cid return base32 CIDv1 CIDs by default instead of base58 (CIDv0 CIDs will continue to use base58). The argument is that:
Proposal here: ipfs/kubo#6220 As far as I can tell, the go-filecoin project doesn't test against any hard-coded base32 CIDv1 CIDs so this shouldn't have any impact (other than maybe changing the CLI output in some cases). However, I figured you'd like to be aware of this change and have a chance to raise objections. If you do have any repos with a bunch of base58 CIDv1 CIDs that you'd like to convert, the following perl script works pretty well (but double check the results, it has false positives):
|
SGTM, ship it! |
No objections, thanks for the heads-up!
…On Tue, 7 May 2019 at 18:34, Steven Allen ***@***.***> wrote:
@whyrusleeping <https://github.com/whyrusleeping> or @anorth
<https://github.com/anorth>,
We'd like to make go-cid return base32 CIDv1 CIDs by default instead of
base58 (CIDv0 CIDs will continue to use base58). The argument is that:
1. Not many users are using CIDv1 yet so this shouldn't be to obvious
of a change.
2. We'd like to switch to base32 by default eventually as it's
case-insensitive. There's no reason not to switch now.
Proposal here: ipfs/kubo#6220
<ipfs/kubo#6220>
------------------------------
As far as I can tell, the go-filecoin project doesn't test against any
hard-coded base32 CIDv1 CIDs so this shouldn't have any impact (other than
maybe changing the CLI output in some cases). However, I figured you'd like
to be aware of this change and have a chance to raise objections.
If you *do* have any repos with a bunch of base58 CIDv1 CIDs that you'd
like to convert, the following perl script works pretty well (but double
check the results, it has false positives):
perl -i -e 'sub base32 { my ($cid) = @_; my $res=`ipfs cid base32 $cid`; chomp $res; return $res; }' -pe 's/\b(z[a-zA-Z0-9]{10,})/"@{[base32($1)]}"/ge' **/*
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#85 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AADMW6Q65CMWDZWZ7UFC4K3PUE5IXANCNFSM4HLB5RJA>
.
|
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.
To remove confusion, we should replace Base58 one in README:
-zdvgqEMYmNeH5fKciougvQcfzMcNjF3Z1tPouJ8C7pc3pe63k
+bafzbeigai3eoy2ccc7ybwjfz5r3rdxqrinwi4rwytly24tdbh6yk7zslrm
And the one in tests (multiple occurences in cid_test.go
):
-zb2rhhFAEMepUBbGyP1k8tGfz7BSciKXP6GHuUeUsJBaK6cqG
+bafkreie5qrjvaw64n4tjm6hbnm7fnqvcssfed4whsjqxzslbd3jwhsk3mm
@lidel I fixed the readme but couldn't find the CIDs you're referring to in the tests. |
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.
@Stebalien my bad, I was looking at master
. All looks good now!
Heads up: breaking change tagged with patch release. People may rely on what cid strings look like (for me it just breaks sharness tests). |
Apologies, I should have bumped the minor. |
ipfs/kubo#6220