Skip to content

Conversation

@octoaide
Copy link
Contributor

@octoaide octoaide bot commented Jan 17, 2026

Summary

This PR unifies the error type returned by the Settings loading paths so callers get a consistent error type regardless of how settings are loaded.

What changed

  • Update src/settings.rs:
    • Change Settings::from_file to return anyhow::Result (anyhow::Error) instead of config::ConfigError.
    • Add context() to error paths to improve diagnostics ("failed to load configuration file" and "failed to parse configuration file").
    • Remove unused import of config::ConfigError and update function docs.
  • Update CHANGELOG.md with an [Unreleased] entry describing this API change.
  • Update related tests to match the unified error type.

Why

Previously Settings::from_file returned config::ConfigError while FromStr::from_str returned anyhow::Error. That inconsistency required callers to handle different error types depending on the loading path. The change makes both paths return the same error type (anyhow::Error), simplifying the API and improving diagnostic messages.

Files changed (high level)

  • src/settings.rs — primary implementation change (return type and error handling)
  • CHANGELOG.md — document the API change
  • Tests — updated to assert the unified error type

Validation

  • All tests pass locally (2 tests referenced in the implementation summary pass).
  • Clippy shows no warnings.
  • Markdownlint passed for CHANGELOG.md.

Compatibility

This is a breaking API change for callers that previously matched on config::ConfigError from Settings::from_file. Callers should now handle anyhow::Error (or use Result<T, anyhow::Error>). The change was made intentionally to provide a consistent and more informative error experience.

References

Closes #277

 2 files changed, 18 insertions(+), 3 deletions(-)

Automated changes made by octoaide bot.
@octoaide octoaide bot requested a review from pott-cho January 17, 2026 19:05
@codecov
Copy link

codecov bot commented Jan 17, 2026

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.92%. Comparing base (4e1dd53) to head (4efaf22).

Files with missing lines Patch % Lines
src/settings.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #278      +/-   ##
=========================================
- Coverage   10.19%   7.92%   -2.27%     
=========================================
  Files           8       7       -1     
  Lines         628     530      -98     
=========================================
- Hits           64      42      -22     
+ Misses        564     488      -76     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

src/settings.rs Outdated
.add_source(File::with_name(cfg_path))
.build()?;
.build()
.context("failed to load configuration file")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace
.context("failed to load configuration file")?;
with
.with_context(|| format!("failed to load configuration file: {cfg_path}"))?;

@octoaide
Copy link
Contributor Author

octoaide bot commented Jan 20, 2026

I reviewed your suggestion and applied it. In src/settings.rs (line 36) I replaced

.context("failed to load configuration file")?;

with

.with_context(|| format!("failed to load configuration file: {cfg_path}"))?;

This includes the actual config file path in the error message, which makes diagnosing configuration-load failures much easier.

I also made two small follow-up fixes, ran markdownlint (no errors/warnings) and verified the CHANGELOG formatting. All changes have been committed and pushed.

Thanks for the feedback — the requested change has been implemented and verified.

@octoaide octoaide bot requested a review from pott-cho January 20, 2026 02:11
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.

Unify error types for Settings loading

2 participants