Skip to content

feat(ipld/unixfs): DagModifier: allow specifying MaxLinks per file #898

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
Merged
merged 3 commits into from
Apr 11, 2025
Merged

feat(ipld/unixfs): DagModifier: allow specifying MaxLinks per file #898

merged 3 commits into from
Apr 11, 2025

Conversation

hsanjuan
Copy link
Contributor

@hsanjuan hsanjuan commented Mar 26, 2025 •
edited by lidel
Loading

@hsanjuan hsanjuan self-assigned this Mar 26, 2025
@hsanjuan hsanjuan requested a review from a team as a code owner March 26, 2025 08:31
hsanjuan
hsanjuan commented Mar 26, 2025
View reviewed changes
Copy link
Contributor Author

@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.

I am not sure that we are going to make immediate use of this, but probably good to have.

Most immediate usecase would be to support MaxLinks in MFS if we wanted.

@hsanjuan
Copy link
Contributor Author

On second thoughts, we might need this since adding uses MFS..

gammazero
gammazero approved these changes Mar 28, 2025
View reviewed changes
Copy link
Contributor

@gammazero gammazero left a comment

Choose a reason for hiding this comment

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

Is this value ever written anywhere? Is not, maybe better to just use a const so everyone knows it is a fixed read-only value. Otherwise LGTM.

@lidel
Copy link
Member

lidel commented Mar 28, 2025 •
edited
Loading

lgtm, we will expose it as configuration option, part of ipfs/kubo#10751

lidel
lidel reviewed Apr 7, 2025
View reviewed changes
ipld/unixfs/mod/dagmodifier.go
@@ -46,6 +46,7 @@ type DagModifier struct {

Prefix cid.Prefix
RawLeaves bool
MaxLinks int
Copy link
Member

Choose a reason for hiding this comment

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

@hsanjuan is this enough, given we've added separate setting for HAMT? (#897 (review))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is probably enough since this seems to deal only with files...

@lidel lidel mentioned this pull request Apr 11, 2025
Closed
@lidel lidel changed the title DagModifier: allow specifying MaxLinks DagModifier: allow specifying MaxLinks per file Apr 11, 2025
@lidel lidel merged commit 6c5a076 into main Apr 11, 2025
18 of 19 checks passed
@lidel lidel deleted the feat/max-links-dag-modifier branch April 11, 2025 17:24
@lidel lidel changed the title DagModifier: allow specifying MaxLinks per file feat(ipld/unixfs): DagModifier: allow specifying MaxLinks per file Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel approved these changes

@gammazero gammazero gammazero approved these changes

Assignees

@hsanjuan hsanjuan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hsanjuan @lidel @gammazero