docs(env): migrate README examples from Result[T, String] to raise - #3813
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the env package documentation examples to align with the library’s preferred failable-API style (T raise E) instead of returning Result[T, String], matching the result_error_return diagnostic guidance.
Changes:
- Migrated the
parse_command,validate_args, andvalidate_and_parse_argsREADME examples fromResult[...]returns toraise Failure. - Updated example call sites to use
try ... catch ... noraise ...for expected-failure handling. - Strengthened the unknown-command example assertion to check the exact error message.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(_) => inspect(false, content="true") | ||
| Err(msg) => inspect(msg.length() > 10, content="true") // Should have error message | ||
| inspect(result, content="Showing help information") | ||
| try parse_command(["program", "invalid"]) |> ignore catch { |
Collaborator
Coverage Report for CI Build 6152Coverage remained the same at 90.876%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
bobzhang
force-pushed
the
hongbo/result-to-raise-docs
branch
from
July 16, 2026 06:57
db5fbde to
9992957
Compare
The result_error_return diagnostic (warning 78) guides failable APIs toward `T raise E` over `Result[T, E]`. Core library code is already clean under this warning on all backends; these README examples were the remaining places teaching the discouraged Result-for-failure style. Rewrite them to raise the standard `Failure` error and use the canonical try/catch/noraise shape for expected failures. Left as-is deliberately (Result-as-ordinary-data carve-out): - builtin/README.mbt.md `divide` — documents the Result type itself - lazy_list/README.mbt.md `maybe_double` — failure stored as data in a lazy structure - strconv/int_test.mbt helpers — feed Result-typed test tables Reviewed by Codex CLI (codex-cli 0.144.1): "No findings; approved for merge... Agree with all deliberate keeps." Signed-off-by: Codex CLI <codex@openai.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bobzhang
force-pushed
the
hongbo/result-to-raise-docs
branch
from
August 18, 2026 09:34
9992957 to
f551576
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the
result_error_returndiagnostic (warning 78, currently off by default): "PreferT raise Efor failable APIs. ReturnResult[T, E]only when success/failure must be represented as ordinary data."Finding
Running
moon check --target all --warn-list +result_error_returnover core produces zero warnings — library code is already fullyraise-based. The only places still teaching the discouraged Result-for-failure style were doc examples inenv/README.mbt.md(Result[T, String]withErr("message")).Change
The three
envexamples (parse_command,validate_args,validate_and_parse_args) now declare-> T raise Failure, raise the standardFailureerror, and demonstrate the canonical expected-failure shape (try ... catch { Failure(msg) => ... } noraise { _ => fail(...) }). The unknown-command assertion got stronger in the process (exact message instead of a length check).Deliberate keeps (Result-as-ordinary-data carve-out)
builtin/README.mbt.mddivide— the section documents theResulttype itselflazy_list/README.mbt.mdmaybe_double— failure cached as data inside a lazy structure (the surrounding prose explains why fallible callbacks don't fit lazy maps)strconv/int_test.mbthelpers — bridge the raising parser into Result-typed test tablesReview
Reviewed by Codex CLI (codex-cli 0.144.1): "No findings; approved for merge. Rewrites are behaviorally equivalent and idiomatic... Assertions preserve intent... Agree with all deliberate keeps."
Signed-off-by: Codex CLI codex@openai.com
Validation
moon checkclean;moon fmtappliedmoon test: 6715 passed, 0 failed (env package 16/16)🤖 Generated with Claude Code