Merge cm updates #436
Merge cm updates #436
Conversation
100mik
force-pushed
the
merge-cm-updates
branch
from
a00b486 to
5999413
Compare
|
Started sorting GVs before storing them as well to make them deterministic |
| @@ -92,7 +76,17 @@ func (a *RecordedApp) usedGKs() (*[]schema.GroupKind, error) { | |||
|
|
|||
| func (a *RecordedApp) UsedGKs() (*[]schema.GroupKind, error) { return a.usedGKs() } | |||
|
|
|||
| func (a *RecordedApp) UpdateUsedGKs(gks []schema.GroupKind) error { | |||
| func (a *RecordedApp) UpdateUsedGVsAndGKs(gvs []schema.GroupVersion, gks []schema.GroupKind) error { | |||
There was a problem hiding this comment.
nit: As this function has grown in size, we can call two private functions 1. func (a *RecordedApp) updateUsedGVs(gvs []schema.GroupVersion) error and other func (a *RecordedApp) updateUsedGKs(gks []schema.GroupKind) error and then call
a.update(func(meta *Meta) {
meta.UsedGVs = uniqGVs
meta.UsedGKs = &uniqGKs
})```
instead of writing everything inside it.
There was a problem hiding this comment.
I would say it is pretty straight forward (with the exact same operations being performed on 2 lists of things)
I did consider it initially but chose not to do that as it would be something like uniqGKs() and uniqGVs() since the functions would just deduplicate the list and the actual update happens in the function itself.
I decided to stick with this because it is more logically straightforward. Leaving this open for discussion.
There was a problem hiding this comment.
im ok with current way.
Reduce the number of calls being made to update app config map