-
Notifications
You must be signed in to change notification settings - Fork 250
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
chore(config)_: rpc providers configuration #6151
Conversation
Jenkins BuildsClick to see older builds (88)
|
8195c81
to
099ebdf
Compare
49c6278
to
0588310
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6151 +/- ##
===========================================
+ Coverage 61.44% 61.47% +0.02%
===========================================
Files 835 842 +7
Lines 110095 110595 +500
===========================================
+ Hits 67651 67991 +340
- Misses 34516 34623 +107
- Partials 7928 7981 +53
Flags with carried forward coverage won't be shown. Click here to find out more.
|
0588310
to
50ee868
Compare
if providers[i].Type == params.UserProviderType { | ||
providers[i].Enabled = enabled | ||
} else { | ||
providers[i].Enabled = !enabled | ||
} |
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.
if providers[i].Type == params.UserProviderType { | |
providers[i].Enabled = enabled | |
} else { | |
providers[i].Enabled = !enabled | |
} | |
providers[i].Enabled = enabled && (providers[i].Type == params.UserProviderType) |
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.
This is the equivalent form:
but I'm not sure it's any easier to understand
providers[i].Enabled = enabled == (providers[i].Type == params.UserProviderType)
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.
Indeed, my bad.
The ==
id definitely worse readable, so better to keep as you did then 👍
@friofry forgot to say: by the end of the review I was so annoyed of the size of the PR that I was tempting to reject it and ask to split in multiple... Looks like it was pretty possible to split it, there's good independency in the new code. |
50ee868
to
1ecae6e
Compare
Sorry for the huge PR, I split it into 2 PRs (configuration + integration). Probably should have split it into more parts. |
5df7c7e
to
e04001a
Compare
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.
Thanks for the fixes 👍
e04001a
to
1c51d12
Compare
* Add rpc_providers table, migration * add RpcProvider type * deprecate old rpc fields in networks, add RpcProviders list * add persistence packages for rpc_providers, networks * Tests (without integration)
1c51d12
to
c1b756f
Compare
Purpose to add some explicitness for:
->
This PR extracts
rpc_provider_persistence
rpc_providers
table, migrationRpcProvider
type + validationThis PR does not change any existing functionality. The next PR will include integration of the new data structure: #6178
Closes #5997