pkg: improve field and variable names

[alexandear]

This PR renames y to instConfig because y is too short for its usage scope, which broke the CodeReviewComments#variable-names statement:

The basic rule: the further from its declaration that a name is used, the more descriptive the name must be

[jandubois]

I don't think yaml is such a good name either. It describes a data format, not what the data is supposed to represent. And the variable has nothing to do with YAML beyond that the struct has been loaded from a YAML file/string.

So if we rename y to something longer, how about instConfig, which is at least descriptive? And it goes well with instName and instDir, which we already use.

[afbjorklund]

Renaming the field is OK, but I don't like not being able to use short variable names (like i, j and k).

Variable names in Go should be short rather than long. This is especially true for local variables with limited scope. Prefer c to lineCount. Prefer i to sliceIndex.

[alexandear]

So if we rename y to something longer, how about instConfig, which is at least descriptive? And it goes well with instName and instDir, which we already use.

Totally agree with you. Renamed to the instConfig.

[alexandear]

Renaming the field is OK, but I don't like not being able to use short variable names (like i, j and k).

It's okay to use i, j, k in for loops where the variable's scope is short, 2-4 lines. Also, one-letter vars are acceptable for receiver names.

It's not okay to use y as the variable name in our case, where the scope is 83 lines.

The Google Go Style Guide contains explanations on variable naming.

[jandubois]

Renaming the field is OK, but I don't like not being able to use short variable names (like i, j and k).

I think it is something to decide on a case-by-case basis. Sometimes the short names work fine, either when their declaration and use case all fits on a single screen, or when the variable is used all the time and not just sparingly.

I could go with cfg for a local variable, but I think using instConfig consistently is better in this case.

Note also how FillDefault continues to use y (and o and d) and has not been modified, because there it is used in every other line, and a longer name would just drown out the field names.

[jandubois]

So this is outside the scope of this PR, but I'm wondering why the BaseDriver has an InstConfig field. Isn't that a duplicate of Instance.Config?

I don't have time to drill into the code right now, but could InstConfig (nee Yaml) be replaced with Instance.Config everywhere? If not, then we should have a comment explaining when/why they can be different.

@balajiv113 Do you remember why you needed both?

[jandubois]

@alexandear Please resolve conflicts.

@afbjorklund Do you still object to the renaming of local variables in this PR, or was that more a general sentiment.

I'll file a separate issue (#2636) to investigate why the BaseDriver type has both a store.Instance and a limayaml.LimaYAML field; that can be sorted out once this PR has been merged.