Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

create internal and private packages #108

Merged

Conversation

schomatis
Copy link
Contributor

First attempt at using the internal package to reduce unnecessary API exposure. After this is merged we can evaluate moving other artifacts from the shard testing if necessary.

@schomatis schomatis self-assigned this Oct 13, 2021
Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

dont use internal, private or similar works better and doesnt punish people for no reason.

@schomatis schomatis force-pushed the schomatis/use-internal-package branch from e42b09f to b9b823b Compare October 14, 2021 14:50
@schomatis schomatis mentioned this pull request Oct 14, 2021
@aschmahmann
Copy link
Contributor

@whyrusleeping this is exclusively used for testing. However, it seems fine to move a utility creating a full HAMT into a package called private and leave people to their own demise.

However, #99 has some changes that I would like to move into the internal package such as a flag for being able to change the hash function used by the HAMT shards (e.g. to the identity function for testing). I do not want people to think that that's something they can do without it being really inconvenient for them. At the moment UnixFS only supports are particular brand of murmur3 and nothing else.

@schomatis could you move the CreateCompleteHAMT into a private package, and then move all of the identity hash pieces into an internal package?

@schomatis schomatis force-pushed the schomatis/use-internal-package branch from b9b823b to 6161095 Compare October 15, 2021 13:05
@schomatis schomatis changed the title move CreateCompleteHAMT to an internal package create internal and private packages Oct 15, 2021
@schomatis schomatis requested review from aschmahmann and removed request for aschmahmann October 15, 2021 13:12
@schomatis
Copy link
Contributor Author

@aschmahmann Refactored into both packages. Can you take another look, please?

hamt/hamt.go Outdated Show resolved Hide resolved
io/directory.go Outdated Show resolved Hide resolved
hamt/util.go Outdated Show resolved Hide resolved
internal/config.go Outdated Show resolved Hide resolved
@schomatis schomatis merged commit 87d3898 into schomatis/directory/unsharding-tests Oct 15, 2021
@schomatis schomatis deleted the schomatis/use-internal-package branch October 15, 2021 22:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants