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

feat: add UpgradeableDirectory #89

Merged
merged 4 commits into from
May 6, 2021
Merged

Conversation

schomatis
Copy link
Contributor

@schomatis schomatis commented May 5, 2021

Toward ipfs/go-mfs#86.

To avoid breaking compatibility we change as little as possible here but probably both HAMTDirectory and BasicDirectory should be made private. (In that case we would export SwitchToSharding from BasicDirectory to the UpgradeableDirectory.)

@schomatis schomatis requested a review from Stebalien May 5, 2021 23:43
@schomatis schomatis self-assigned this May 5, 2021
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

This is a reasonable first step. I can't think of anything it's likely to break at the moment.

// if we should switch to HAMTDirectory according to global option(s).
func (d *UpgradeableDirectory) AddChild(ctx context.Context, name string, nd ipld.Node) error {
if UseHAMTSharding {
if basicDir, ok := d.Directory.(*BasicDirectory); ok {
Copy link
Member

Choose a reason for hiding this comment

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

nit: check both types and fail if we have some strange third type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, will address in upcoming PR, i already have some commits on top of this code

Copy link
Member

Choose a reason for hiding this comment

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

(note: for me, "nit" means "I don't really care but I had this random thought while reading this code"; totally optional)

@schomatis schomatis merged commit 28e86c5 into master May 6, 2021
@schomatis schomatis deleted the schomatis/feat/upgradeable-dir branch May 6, 2021 22:40
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.

2 participants