Skip to content

Conversation

@MichaelMure
Copy link
Contributor

It seems to me that this section of the code is unnecessary as a similar setup would be done in the adder already.

Additionally that would avoid using a test mock, which is questionable to begin with.

It seems to me that this section of the code is unnecessary as a similar setup would be done [in the adder](https://github.com/ipfs/kubo/blob/a7c65184976e8717ac23d7efaa5b0d477ad15deb/core/coreunix/add.go#L86-L101) already.

Additionally that would avoid using a test mock, which is questionable to begin with.
@MichaelMure MichaelMure requested a review from a team as a code owner October 20, 2023 11:59
@MichaelMure MichaelMure marked this pull request as draft October 20, 2023 14:33
@MichaelMure
Copy link
Contributor Author

Back to draft as this fails the CI. My assumption was obviously wrong, but I'll use that PR to discuss.

If I get that right:

  • with --only-hash, we still want to simulate what would happen with MFS, I assume to return the proper hash
  • to do so, we need a MFS with a root dir around
  • the problem with the only-hash code path is that we push blocks to null datastore, so there is no MFS root stored there
  • to workaround the problem, another DagService is created with an in-memory datastore, specifically to store that empty MFS root, to be pulled later in the adder

Now, I don't understand why we do this ? Why not just wrap in an empty unixfsdir? Why even wrap?

Would there be a way to simplify that code?

@BigLep BigLep mentioned this pull request Nov 9, 2023
11 tasks
@gammazero gammazero added the skip/changelog This change does NOT require a changelog entry label Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip/changelog This change does NOT require a changelog entry

Projects

No open projects
Status: 🔎 In Review

Development

Successfully merging this pull request may close these issues.

2 participants