rename most of the sandbox vars, params, and references #15
Conversation
MatousJobanek
requested review from
xcoulon,
alexeykazakov,
rajivnathan,
ranakan19,
sbryzak,
mfrancisc and
drpaneas
as code owners
pkg/configuration/configuration.go
Outdated
| @@ -110,92 +110,92 @@ type ClusterNamespaces map[string]string | |||
|
|
|||
| // LoadClusterAccessDefinition loads ClusterAccessDefinition object from the config file and checks that all required parameters are set | |||
| func LoadClusterAccessDefinition(term ioutils.Terminal, clusterName string) (ClusterAccessDefinition, error) { | |||
| sandboxUserConfig, _, err := Load(term) | |||
| ksctlConfig, _, err := Load(term) | |||
There was a problem hiding this comment.
cfg would be enough here. It's a shorter name, and I guess that the ksctl prefix is superfluous here
There was a problem hiding this comment.
that variable name is used at multiple places, I would keep it as it is to make it consistent and self-explanatory.
if you insist on making it shorter, I could rename the occurrences to ksctlCfg, but I'm not sure if 3 chars extra is a big deal.
There was a problem hiding this comment.
as I said, I don't really see the value of the ksctl prefix but I will not insist on this ;)
pkg/configuration/configuration.go
Outdated
| type ClusterConfig struct { | ||
| ClusterAccessDefinition | ||
| AllClusterNames []string | ||
| ClusterName string | ||
| Token string | ||
| SandboxNamespace string | ||
| KubeSawNamespace string |
There was a problem hiding this comment.
the var name remains a bit ambiguous IMO. Is it the toolchain-host-operator namespace (as we still call it now)? What about Namespace (very generic, I admit) or OperatorNamespace instead?
There was a problem hiding this comment.
yeah, it's a bit tricky since it can be either toolchain-host-operator or toolchian-member-operator ns.
I was thinking about:
KubeSawOperatorNamespaceKubeSawOpNamespaceKubeSawOperatorNsKubeSawOpNs
to make it clear which type of operator we are talking about.
But yeah, OperatorNamespace could be fine.
There was a problem hiding this comment.
or ksOperatorNamespace
There was a problem hiding this comment.
having a comment on the field to explain that its value can be either toolchain-host-operator or toolchain-member-operator (depending on the type of cluster we're dealing with) would be nice, too.
There was a problem hiding this comment.
renamed to operatorNamespace in 3abbff6
There was a problem hiding this comment.
added comment: 7c66812
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #15 +/- ##
=======================================
Coverage 65.28% 65.28%
=======================================
Files 38 38
Lines 1976 1976
=======================================
Hits 1290 1290
Misses 528 528
Partials 158 158
|
renaming most of the variables, parameters, and references containing "sandbox" word to either KubeSaw or ksctl related names.