Remove extra property addition to Kilnfile #469
Conversation
Print releases not glazed Rename float_always to skip_glaze
|
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story. The labels on this github issue will be updated when the story is started. |
| @@ -36,8 +36,10 @@ func (kf *Kilnfile) BOSHReleaseTarballSpecification(name string) (BOSHReleaseTar | |||
|
|
|||
| func (kf *Kilnfile) Glaze(kl KilnfileLock) error { | |||
| kf.Stemcell.Version = kl.Stemcell.Version | |||
| var SkipGlazeReleases []string | |||
There was a problem hiding this comment.
please lowercase the first character in the identifier
|
|
||
| // FloatAlways when does not override version constraint. | ||
| // It skips locking it during Kilnfile.Glaze. | ||
| FloatAlways bool `yaml:"float_always,omitempty"` | ||
| FloatAlways bool `yaml:"skip_glaze,omitempty"` |
There was a problem hiding this comment.
I recall @staylor14 asking for the name "float_always". I don't recall the exact reasoning. I remember liking it. I believe it had something to do with describing the behavior in connection with the deglaze policy names.
My best argument for the field "float_always" is I always sing a few lines of Janelle Monáe's "Float" when I see the field "always_float" and it makes me happy.
There was a problem hiding this comment.
"float_always" could confuse the component teams to think that it is equivalent to the version constraint "*" and they may end up adding this property to their releases.
There was a problem hiding this comment.
Removing that source of confusion makes sense to me. I think I like the change now.
I'll reach out to @staylor14 to see if he has a strong opinion.
There was a problem hiding this comment.
So, "*" is a kind of float. glaze will pin that. "1.*" is also a float. glaze will pin that.
In my head, a Component team is usually asking for some kind of float, so the directive that made sense to me in the face of a glaze command was float_always.
If it makes more sense to more people to put this in terms of the command (glaze) rather than the effect (floating), that's cool with me, BUT!
I do not like booleans that are named for negation (e.g. do_not_do), nor do I like having a difference between internal and external names (FloatAlways vs skip_glaze). I propose we re-do both names: AllowGlaze and allow_glaze. Note that this will require a flip to the actual logic in-play, but that's to be expected because the semantic meaning of the words has now flipped (float_always=true meant "never glaze", and now allow_glaze=true will mean the opposite).
How'z about them apples? Who likes the apples?
There was a problem hiding this comment.
Also, is the phrase "semantic meaning" redundant? I think it's redundant. Yeah, it's redundant. Redundant.
|
Closing this as we decided to change the approach. |
Print releases not glazed
Rename float_always to skip_glaze