Rearrange step schema #182
Conversation
ghost
assigned 
carolynvs
force-pushed
the
rearrange-mixin-schema
branch
from
fbf339f
to
56fca27
Compare
|
Heh, this PR is why I opened #180. 😀 |
pkg/config/manifest.go
Outdated
| children := s.Data[mixinName] | ||
| d, ok := children.(map[interface{}]interface{})["description"] | ||
| if !ok { | ||
| return "", errors.Errorf("mixin (%s) missing description", mixinName) |
There was a problem hiding this comment.
Q: Wondering if step should be included in this and the error message below... mixin step (%s) missing..., etc.?
I'm thinking both for clarification and if/when a given mixin has multiple step entries for an action (well, perhaps the latter is a bad practice/rare... though I could perhaps see it occurring with exec). Then again, not sure how we'd identify which mixin step needed fixing if there were more than one step using the same mixin and missing a description...
Mostly thinking out loud here... feel free to ignore :)
There was a problem hiding this comment.
For now it's a very shallow/poor yaml validation going on here. Let me spruce up that error message.
Once we have real schema validation being applied (which comes after being able to say what our schema is) then we can let that report back what's wrong. The errors coming out of that will give the line number of which step is incorrect.
This moves a step's schema entirely under the mixin. So description and outputs are now under the mixin.
This makes it easier for the mixin to control the schema of the outputs. For example, helm outputs specify secret keys for where to find and publish secrets as outputs. Another mixin like exec, doesn't have outputs at all, and this lets it express that.
Moving description under, simply makes it cleaner and easier to deal with merging schemas.
I've done a sweep and updated all the docs and testdata to match the new schema.