Build improvements #623
Build improvements #623
Conversation
pkg/builder/builder_steps.go
Outdated
|
|
||
| func RegisterStep(steps ...Step) { | ||
| for _, step := range steps { | ||
| StepsByID[step.ID()] = step |
There was a problem hiding this comment.
now that the id has a real meaning wondering if we need to add some tests/check to avoid overriding a step by mistake
There was a problem hiding this comment.
Yes I thought about it but got somehow a bit lazy 🙈. I guess we can fail-fast and panic because step overriding should not happen.
There was a problem hiding this comment.
yep but a test would help too :) just to avoid to find the problem at runtime
There was a problem hiding this comment.
I've added panicking and unit testing, and updated the builder integration tests to cover all the packages.
pkg/builder/s2i/s2i.go
Outdated
|
|
||
| var Steps = steps{ | ||
| Publisher: builder.NewStep( | ||
| "publisher/s2i", |
There was a problem hiding this comment.
maybe we should find a better naming convention for steps IDs like s2i/publisher to reduce the risk of overriding steps f
There was a problem hiding this comment.
@astefanutti wondering if you have time to fix this too before merging
There was a problem hiding this comment.
Yes, better doing it while we are at it.
There was a problem hiding this comment.
The step IDs are now generated from the structs and automatically set on each step with reflection.
|
Great work! 👍 |
No description provided.