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.

Sign up for GitHub

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

Jump to bottom

mount: switch over to the CoreAPI #6602

Merged
merged 3 commits into from Mar 7, 2020
Merged

mount: switch over to the CoreAPI #6602

merged 3 commits into from Mar 7, 2020

Conversation

Stebalien
Copy link
Member

  1. Fix resolution of sharded directories.
  2. Slowly kill off users of the IpfsNode object.

@Stebalien
Copy link
Member Author

The second two commits just delete some code.

@aschmahmann
Copy link
Contributor

This interop test is now failing: ipns locally using the same repo across implementations should publish an ipns record to a go daemon and resolve it using a js daemon through the reuse of the same repo - should publish an ipns record to a go daemon and resolve it using a js daemon through the reuse of the same repo

@Stebalien Stebalien force-pushed the feat/coreapi-mount branch 2 times, most recently from 32b4caf to e5c5fe7 Compare January 30, 2020 22:46
@Stebalien
mount: switch over to the CoreAPI
76b1546 
1. Fix resolution of sharded directories.
2. Slowly kill off users of the IpfsNode object.
@Stebalien
cmds/tar: use the coreapi
8643d94
@Stebalien
resolve: kill off buggy resolve function
97bc89d 
This resolve function assumed that all paths were of the same type (ipfs, ipld,
etc.). The CoreAPI does a much better job.
@Stebalien Stebalien requested review from lidel and removed request for aschmahmann March 2, 2020 17:24
@Stebalien
Copy link
Member Author

This interop test is now failing

Not anymore? Not sure what this was, but we've fixed some interop issues since then.

lidel
lidel reviewed Mar 5, 2020
View reviewed changes
Copy link
Member

@lidel lidel left a comment •
edited

Choose a reason for hiding this comment

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

Not sure if I understand full context of this migration, but the tests added in #609 (mostly gateway, but it includes 58a95f3) will keep us safe when we decide to do a similar switch for /ipns/ resolve in core/corehttp.
(sould be a separate PR tho)

@Stebalien
Copy link
Member Author

Context:

  • Better (unified) path resolution.
  • Deleted code.
  • Works towards extracting this code into a separate package.

hsanjuan
hsanjuan approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@hsanjuan hsanjuan left a comment

Choose a reason for hiding this comment

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

Code-wise, I see mostly renamings and nothing specially tricky compared to what was there before so lgtm...

ribasushi
ribasushi approved these changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@ribasushi ribasushi left a comment

Choose a reason for hiding this comment

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

LGTM

@Stebalien Stebalien merged commit 1ae6986 into master Mar 7, 2020
@hsanjuan hsanjuan deleted the feat/coreapi-mount branch March 9, 2020 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel left review comments

@ribasushi ribasushi ribasushi approved these changes

@hsanjuan hsanjuan hsanjuan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Stebalien @aschmahmann @lidel @ribasushi @hsanjuan