Conversation
|
Sorry, yet more of me not paying proper attention and/or trying to repro things I broke this in #473 with some code which went through heavy duty testing, but only on Cosmos DB Until I merge and release, you can validate this fix via:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the eqx dump command failed when using non-Cosmos stores (specifically Dynamo) due to eager evaluation of Cosmos-specific arguments. The fix adds the Last attribute to store type parameters and refactors the Cosmos argument initialization to be lazy. Additionally, the tool's target framework is upgraded from .NET 6.0 to .NET 8.0.
Changes:
- Added
Lastattribute to Cosmos and Dynamo store parameters in DumpParameters to fix argument parsing - Refactored CosmosArgs from eager member val to lazy evaluation within Connect() method
- Upgraded target framework from net6.0 to net8.0
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/Equinox.Tool/Program.fs | Fixed DumpParameters to add Last attribute and refactored CosmosArgs initialization to be lazy |
| tools/Equinox.Tool/Equinox.Tool.fsproj | Updated target framework from net6.0 to net8.0 with corresponding comment update |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | [<AltCommandLine "-E"; Unique >] EventsOnly | ||
| | [<CliPrefix(CliPrefix.None)>] Cosmos of ParseResults<Store.Cosmos.Parameters> | ||
| | [<CliPrefix(CliPrefix.None)>] Dynamo of ParseResults<Store.Dynamo.Parameters> | ||
| | [<CliPrefix(CliPrefix.None); Last>] Cosmos of ParseResults<Store.Cosmos.Parameters> |
There was a problem hiding this comment.
Last does not appear to make a functional difference, but there's no reason for Cosmos or Dynamo to be different (chances are all can be safely removed but leaving for another day)
Works great! |
Fixes bug introduced in #473 (assumption of Cosmos in a
member val)via #479 thanks to @njlr