Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

feat: OptionalString and UnixFSShardingSizeThreshold #149

Merged
merged 5 commits into from
Oct 28, 2021

Conversation

aschmahmann
Copy link
Contributor

Also comes with a type definition for optional strings.

Copy link
Member

@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.

OptionalString lgtm.

To avoid writing size parsers in multiple places, we may want to add Size type similar to Duration that takes care of serializing and deserializing bytes to human-readable form like "10GB", but this could be added in the future if we run out of time this week.

@aschmahmann
Copy link
Contributor Author

We can use go-humanize for the conversion since we already use it for this job in go-ipfs. e.g. https://github.com/ipfs/go-ipfs/blob/ef866a1400b3b2861e5e8b6cc9edc8633b890a0a/core/corerepo/stat.go#L81

Copy link
Member

@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.

Tested with ipfs/kubo#8527 and works as expected.

Note to self: we need OptionalString in other places, so let's land this as-is and add dedicated type for humanized byte sizes in a separate PR.

@lidel lidel self-assigned this Oct 28, 2021
@lidel lidel changed the title feat: internal unixfs sharding threshold feat: OptionalString and UnixFSShardingSizeThreshold Oct 28, 2021
@lidel lidel marked this pull request as ready for review October 28, 2021 15:58
@lidel
Copy link
Member

lidel commented Oct 28, 2021

Merging to unblock #150 which in turn will unblock sharness tests in libp2p update

@lidel lidel merged commit 58ff619 into master Oct 28, 2021
@lidel lidel deleted the feat/internal-unixfs-sharding-threshold branch October 28, 2021 20:55
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants