Skip to content
This repository was archived by the owner on Jan 28, 2025. It is now read-only.

fix: solution.Error() returns untyped nil when SAT#140

Merged
joelanford merged 1 commit intooperator-framework:mainfrom
joelanford:fix-solution-error
Aug 15, 2023
Merged

fix: solution.Error() returns untyped nil when SAT#140
joelanford merged 1 commit intooperator-framework:mainfrom
joelanford:fix-solution-error

Conversation

@joelanford
Copy link
Copy Markdown
Member

Fixes #139

@joelanford joelanford requested a review from a team as a code owner August 14, 2023 12:43
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 14, 2023

Codecov Report

Merging #140 (1b37541) into main (4c15c99) will increase coverage by 0.13%.
The diff coverage is 100.00%.

❗ Current head 1b37541 differs from pull request most recent head f58565c. Consider uploading reports for the commit f58565c to get more accurate results

@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
+ Coverage   61.90%   62.03%   +0.13%     
==========================================
  Files          16       16              
  Lines         567      569       +2     
==========================================
+ Hits          351      353       +2     
  Misses        198      198              
  Partials       18       18              
Files Changed Coverage Δ
pkg/deppy/solver/solver.go 84.21% <100.00%> (+0.87%) ⬆️

Comment thread pkg/deppy/solver/solver.go Outdated
Comment on lines +27 to +29
if s.err == nil {
return nil
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should just use error interface for s.err instead a specific type. This way we avoid having this strange if and still can avoid nil not being nil issue.

diff --git a/pkg/deppy/solver/solver.go b/pkg/deppy/solver/solver.go
index d60036f..f8758f5 100644
--- a/pkg/deppy/solver/solver.go
+++ b/pkg/deppy/solver/solver.go
@@ -16,7 +16,7 @@ import (
 // A successful execution of the solver can still end in an error when no solution can
 // be found.
 type Solution struct {
-       err       deppy.NotSatisfiable
+       err       error
        selection map[deppy.Identifier]deppy.Variable
        variables []deppy.Variable
 }

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done! Wasn't sure if that change would have knock-on effects. Looks like it compiles and tests pass though, so I assume all good.

Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using solver.Solution.Error() is awkward and unintuitive

2 participants