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

refactor: Improve clarity of ConfigError variants #936

Merged
merged 2 commits into from
Feb 20, 2025

Conversation

cstepanian
Copy link
Contributor

@cstepanian cstepanian commented Feb 19, 2025

Resolves #931

Add new variants to replace all existing uses of Unspecified, other than serde_json parse errors:

  • InternalError
  • FileNotFound
  • ParseError
  • EnvVarNotSet
  • MissingProgram

@cstepanian cstepanian force-pushed the cstepanian/config_error branch 3 times, most recently from 2875c9e to f864c0f Compare February 19, 2025 21:23
@j-lanson
Copy link
Collaborator

@alilleybrinker tagging you to take a look at this since we can't walk back changes to the protocol definition once added.

@cstepanian cstepanian force-pushed the cstepanian/config_error branch 2 times, most recently from 2836d05 to 1806751 Compare February 19, 2025 21:38
@cstepanian cstepanian requested a review from j-lanson February 19, 2025 21:39
@cstepanian cstepanian force-pushed the cstepanian/config_error branch from 1806751 to 4204d68 Compare February 19, 2025 21:55
@cstepanian cstepanian requested a review from j-lanson February 19, 2025 21:57
@cstepanian cstepanian force-pushed the cstepanian/config_error branch from 4204d68 to 92241d9 Compare February 19, 2025 21:57
Resolves #931

Add new variants to replace all existing uses of `Unspecified`, other than `serde_json` parse errors:
- `InternalError`
- `FileNotFound`
- `ParseError`
- `EnvVarNotSet`
- `MissingProgram`

Signed-off-by: Cal Stepanian <[email protected]>
@cstepanian cstepanian force-pushed the cstepanian/config_error branch from 92241d9 to 69efa8c Compare February 19, 2025 22:28
@cstepanian cstepanian requested a review from j-lanson February 19, 2025 22:33
Copy link
Collaborator

@j-lanson j-lanson left a comment

Choose a reason for hiding this comment

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

LGTM!

@j-lanson j-lanson merged commit 8b3d8e0 into main Feb 20, 2025
11 checks passed
@cstepanian cstepanian deleted the cstepanian/config_error branch February 21, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Ensure that ConfigError enum is sufficiently expressive
2 participants