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

docs(ipfs.add): flag behavior around cidv1 #8457

Merged
merged 2 commits into from
Oct 14, 2021
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 24, 2021

Closes #8396

@Jorropo would this make things clean enough?

@lidel lidel added the topic/docs Documentation label Sep 24, 2021
@lidel lidel requested a review from aschmahmann September 24, 2021 16:43
@Jorropo
Copy link
Contributor

Jorropo commented Sep 24, 2021

@lidel you got the things the wrong way around.

My issue is that CIDv1 implies raw leaf unless the user specify it otherwise. (CIDv1 -> Rawleaves)
Your PR explain that raw leaf implies CIDv1 which is false, I mean just test it raw leaves on CIDv0 just works. (Rawleaves -> CIDv1 which is false)

I hope this clears it out :)

Copy link
Contributor

@Jorropo Jorropo left a comment

Choose a reason for hiding this comment

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

See the comment

@Jorropo
Copy link
Contributor

Jorropo commented Sep 27, 2021

By the way, when I've said : "Your PR explain that raw leaf implies CIDv1 which is false" that is partialy wrong.
If the target file is smaller than the blocksize CIDv1 is implied, else (if the file gets chunked) CIDv0 is implied. Which makes no sense to me, just pick one and stick with it.

core/commands/add.go Outdated Show resolved Hide resolved
core/commands/add.go Outdated Show resolved Hide resolved
Co-authored-by: Adin Schmahmann <adin.schmahmann@gmail.com>
Copy link
Member Author

@lidel lidel left a comment

Choose a reason for hiding this comment

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

Applied suggestions from review and dropped (experimental) from cid-version and raw-leaves. CIDv1 has been used in production for more than a year now, no need to keep this until it becomes the default.

I sympathize with @Jorropo that behavior is confusing, but realistically the best we can do to solve it is to continue work towards CIDv1 as default (#4143, we have ipfs/fs-repo-migrations#95 scheduled to land by EOY).

Let's merge this as-is and invest time towards CIDv1.

@lidel lidel requested a review from aschmahmann October 11, 2021 18:40
@BigLep BigLep merged commit 8836a62 into master Oct 14, 2021
@BigLep BigLep deleted the docs/raw-leaves-in-cidv1 branch October 14, 2021 15:42
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/docs Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ipfs add --raw-leaves is enabled if CIDv1 is set
5 participants