Change SimApp to have public keepers #4894
Conversation
colin-axner
requested review from
alessio,
alexanderbez,
fedekunze,
jackzampolin and
rigelrozanski
as code owners
Codecov Report
@@ Coverage Diff @@
## master #4894 +/- ##
==========================================
+ Coverage 54.03% 54.04% +<.01%
==========================================
Files 269 269
Lines 17130 17141 +11
==========================================
+ Hits 9256 9263 +7
- Misses 7186 7190 +4
Partials 688 688 |
simapp/app.go
Outdated
| @@ -57,7 +57,7 @@ var ( | |||
| ) | |||
|
|
|||
| // module account permissions | |||
| maccPerms = map[string][]string{ | |||
| MaccPerms = map[string][]string{ | |||
There was a problem hiding this comment.
I don't see the need of making this public as it's only used on this file
There was a problem hiding this comment.
if an external module want to have their own module account's they need to recreate the supply keeper with their added module accounts. It can be useful to reuse the existing mapping rather than duplicating it with some extra additions. I could keep this private and add a getter function?
There was a problem hiding this comment.
true true. It's fine as is bc it's not internally part of the SimApp
There was a problem hiding this comment.
Since this map is now public, it can be modified; can be dangerous. I'd rather see it private and expose a getter that dups it.
simapp/app.go
Outdated
| @@ -57,7 +57,7 @@ var ( | |||
| ) | |||
|
|
|||
| // module account permissions | |||
| maccPerms = map[string][]string{ | |||
| MaccPerms = map[string][]string{ | |||
There was a problem hiding this comment.
Since this map is now public, it can be modified; can be dangerous. I'd rather see it private and expose a getter that dups it.
| @@ -0,0 +1 @@ | |||
| #4893 Change SimApp keepers to be public and add getter functions for keys and codec | |||
There was a problem hiding this comment.
Let's remove this file and copy contents to CHANGELOG.md 👍
fixes #4893
Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/)Added a relevant changelog entry:
clog add [section] [-t <tag>] [-m <msg>]Re-reviewed
Files changedin the github PR explorerFor Admin Use: