fix: Remove required Close from database.Migrate #25
Conversation
Codecov Report
@@ Coverage Diff @@
## main #25 +/- ##
==========================================
+ Coverage 71.54% 71.93% +0.38%
==========================================
Files 37 37
Lines 1311 1304 -7
Branches 7 7
==========================================
Hits 938 938
+ Misses 299 294 -5
+ Partials 74 72 -2
Continue to review full report at Codecov.
|
| if srcErr != nil { | ||
| return xerrors.Errorf("close source: %w", err) | ||
| } | ||
| if dbErr != nil { | ||
| return xerrors.Errorf("close database: %w", err) | ||
| } |
There was a problem hiding this comment.
Ah, so whoever calls Migrate needs to be responsible for cleaning up the db? May be worth noting in the comment for that function - I'd assume if I'm just passing in the dbName that this would be handled for me.
There was a problem hiding this comment.
I'll remove dbName as an argument actually. It's primarily used for debug logging, but isn't required at all.
Then it'll make more sense that this doesn't close the conn.
This caused an unfortunate requirement of two database connections to instantiate a single instance. Now, the same connection can be used for migrations and querying.