8000 Chore: make pathfunc optional for dagop wrappers by alexcb · Pull Request #10568 · dagger/dagger · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Chore: make pathfunc optional for dagop wrappers #10568

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
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Chore: make pathfunc optional for dagop wrappers #10568

wants to merge 1 commit into from

Conversation

alexcb
Copy link
Collaborator
@alexcb alexcb commented Jun 11, 2025

Refactors the dagop wrappers to make the path function optional. When ommitted, it will now default to the parent's dir or file value.

@alexcb
Chore: make pathfunc optional for dagop wrappers
71d9b03 
Refactors the dagop wrappers to make the path function optional. When
ommitted, it will now default to the parent's `dir` or `file` value.

Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
@alexcb
Copy link
Collaborator Author
alexcb commented Jun 11, 2025

The one thing I am a bit unsure of, is previously the default Path value was hard-coded to either / or file, depending on the type that's being returned; however, this changes it to be dependant on the parent type, which seems a bit odd.

e.g. c.Directory("some-dir").File("a") will have the default path set to some-dir rather than file; both of these seem wrong since we would want some-dir/a.

Then there's also the case of c.Directory("foo").Directory("bar"), which would set the default to foo rather than bar.

Maybe instead we should require a pathfunc be explicitly set in all cases? That might prevent future bugs since we'll have to be explicit about the behaviour we want.

@github-actions GitHub Actions
Copy link
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
kind/stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@alexcb
0