Skip to content

feat(filesystem):Add error-code other#894

Merged
ricochet merged 2 commits intomainfrom
other-for-non-exhaustive
Mar 13, 2026
Merged

feat(filesystem):Add error-code other#894
ricochet merged 2 commits intomainfrom
other-for-non-exhaustive

Conversation

@ricochet
Copy link
Contributor

@ricochet ricochet commented Mar 3, 2026

Part of #892

@ricochet ricochet requested a review from a team as a code owner March 3, 2026 00:42
@github-actions github-actions bot added the P-filesystem Proposal: wasi-filesystem label Mar 3, 2026
vados-cosmonic
vados-cosmonic previously approved these changes Mar 3, 2026
Copy link
Contributor

@vados-cosmonic vados-cosmonic left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@ricochet ricochet marked this pull request as draft March 3, 2026 14:35
@github-actions github-actions bot added P-cli Proposal: wasi-cli dependencies Pull requests that update a dependency file labels Mar 3, 2026
@ricochet
Copy link
Contributor Author

ricochet commented Mar 3, 2026

Thank you for the review! While this is a small tweak, I want to make sure we first have consensus on the precedent (specifically changing to variant with other(option<string>)): #892 (comment)

@vados-cosmonic vados-cosmonic marked this pull request as ready for review March 4, 2026 17:21
@vados-cosmonic vados-cosmonic requested a review from a team as a code owner March 4, 2026 17:21
@vados-cosmonic
Copy link
Contributor

Ah the variant with the option<string> is way better -- thanks. People not being able to immediately know what the error was would be quite frustrating.

@ricochet ricochet force-pushed the other-for-non-exhaustive branch from a1ddd87 to 15d8767 Compare March 12, 2026 20:49
@ricochet ricochet requested a review from a team as a code owner March 12, 2026 20:49
@ricochet ricochet force-pushed the other-for-non-exhaustive branch from 15d8767 to ad4ec7c Compare March 12, 2026 20:55
Copy link
Member

@lukewagner lukewagner left a comment

Choose a reason for hiding this comment

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

lgtm2 (as the pattern we're using elsewhere)

@ricochet ricochet merged commit b7ee9fe into main Mar 13, 2026
2 checks passed
@ricochet ricochet deleted the other-for-non-exhaustive branch March 13, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file P-cli Proposal: wasi-cli P-filesystem Proposal: wasi-filesystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants