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

pkg/tool/exec: add .Default() to struct env value #1787

Closed
wants to merge 1 commit into from

Conversation

addreas
Copy link

@addreas addreas commented Jul 11, 2022

Encountered an error that was quite confusing: invalid environment variable value "perfectly-reasonable-string", looking into what caused that I ended up with this little diff.

@addreas addreas requested a review from cueckoo as a code owner July 11, 2022 12:07

Partially verified

This commit is signed with the committer’s verified signature.
spydon’s contribution has been verified via GPG key.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
Signed-off-by: Andreas Mårtensson <[email protected]>
@addreas addreas force-pushed the exec-run-env-default branch from b0c4e9d to 1a23806 Compare July 12, 2022 06:49
@mvdan
Copy link
Member

mvdan commented Apr 10, 2023

Thank you for contributing, and apologies that we hadn't replied yet :)

Per the contributing docs, if this is meant to fix a bug, please raise an issue following the bug template first. That way we can be sure that we're on the same page and can reproduce the issue before we consider a fix.

@addreas
Copy link
Author

addreas commented Apr 10, 2023

I've left the code that required this behind so I would be more likely to close this PR than create a bug report. The test change should demonstrate the motivation for this, though.

@mvdan mvdan self-assigned this Dec 30, 2023
@mvdan
Copy link
Member

mvdan commented Dec 30, 2023

Thanks - now imported to Gerrit as https://review.gerrithub.io/c/cue-lang/cue/+/1174128.

@mvdan
Copy link
Member

mvdan commented Jan 9, 2024

It turns out that an issue was filed for this half a year ago (#2572) and #2764 was just sent as an alternative PR, which also fixes this for lists and has more tests, so I'll go with that one assuming it's okay with you. Thank you for contributing nonetheless!

@mvdan mvdan closed this Jan 9, 2024
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.

None yet

2 participants