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.

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

feat: Enable to use defaultDagBuilder with custom builders #413

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

clostao
Copy link

@clostao clostao commented Sep 4, 2024

I'm trying to add some additional optional metadata to directory's IPLD objects having an almost equal IPLD object as the IPFS standard. For this reason, we would want to use the defaultDagBuilder but customising just how file and directories IPLD objects are formed.

For achieving this I added two additional fields to DagBuilderOptions & ImporterOptions that would override the dirBuilder & fileBuilder if that option is provided.

@clostao clostao changed the title Enable to use defaultDagBuilder with custom fileBuilder or dirBuilder update: Enable to use defaultDagBuilder with custom fileBuilder or dirBuilder Sep 4, 2024
@clostao clostao changed the title update: Enable to use defaultDagBuilder with custom fileBuilder or dirBuilder feat: Enable to use defaultDagBuilder with custom fileBuilder or dirBuilder Sep 4, 2024
@clostao clostao changed the title feat: Enable to use defaultDagBuilder with custom fileBuilder or dirBuilder feat: Enable to use defaultDagBuilder with custom builders Sep 4, 2024
Copy link
Member

@achingbrain achingbrain left a comment

Choose a reason for hiding this comment

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

This looks good. Please can you:

  1. Revert the formatting changes
  2. Add some tests to ensure there are no regressions in future

packages/ipfs-unixfs-importer/tsconfig.json Show resolved Hide resolved
@clostao
Copy link
Author

clostao commented Sep 15, 2024

I pushed the requested changes lmk if I can furtherly help

@clostao
Copy link
Author

clostao commented Oct 1, 2024

Any news on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants