-
Notifications
You must be signed in to change notification settings - Fork 200
deployer: reorganize with subpackages #1064
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1064 +/- ##
==========================================
- Coverage 33.82% 33.42% -0.40%
==========================================
Files 46 51 +5
Lines 3752 3752
==========================================
- Hits 1269 1254 -15
- Misses 2338 2355 +17
+ Partials 145 143 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| type OpDeployer struct { | ||
| binaryPath string | ||
| merger StateMerger | ||
| merger state.StateMerger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly, but there's an opportunity to rename here so we have e.g. state.Merger a la Effective Go guidelines https://go.dev/doc/effective_go#package-names . Same goes for many other symbols that have "state" in the name.
| package opaque_map | ||
|
|
||
| type OpaqueMap map[string]any |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could call the package opaque and the map Map ?
| ) | ||
| } | ||
|
|
||
| func (om OpaqueState) GetNumChains() (int, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we are considering Effective Go conventions, we might also want to drop all of the Get- and Read- prefixes from these methods?
This pr restructures the
ops/internal/deployerpackage, adding some subpackages for better organization. This is one step towards addressing the issue identified here where its difficult to add a new state version. Follow-up steps will be to extract more duplicate code from thestate/vX.gofiles into thestate.state.gofile. Instead of having a flatdeployerpackage, we now have the following dir structure: