ipfs / go-cid

Content ID v1 implemented in go
MIT License
157 stars 47 forks source link

default cidv1 to base32 #85

Closed Stebalien closed 5 years ago

Stebalien commented 5 years ago

https://github.com/ipfs/go-ipfs/issues/6220

Stebalien commented 5 years ago

@whyrusleeping or @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: https://github.com/ipfs/go-ipfs/issues/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' **/*
whyrusleeping commented 5 years ago

SGTM, ship it!

anorth commented 5 years ago

No objections, thanks for the heads-up!

On Tue, 7 May 2019 at 18:34, Steven Allen notifications@github.com 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/go-ipfs#6220 https://github.com/ipfs/go-ipfs/issues/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 https://github.com/ipfs/go-cid/pull/85#issuecomment-489987922, or mute the thread https://github.com/notifications/unsubscribe-auth/AADMW6Q65CMWDZWZ7UFC4K3PUE5IXANCNFSM4HLB5RJA .

Stebalien commented 5 years ago

@lidel I fixed the readme but couldn't find the CIDs you're referring to in the tests.

hsanjuan commented 5 years ago

Heads up: breaking change tagged with patch release. People may rely on what cid strings look like (for me it just breaks sharness tests).

Stebalien commented 5 years ago

Apologies, I should have bumped the minor.