Re-implement init UI using terminal UI components (based on PTerm.sh)#400
Re-implement init UI using terminal UI components (based on PTerm.sh)#400
init UI using terminal UI components (based on PTerm.sh)#400Conversation
87e5d67 to
8b06e99
Compare
ffce808 to
16b8409
Compare
9ac514b to
67beab0
Compare
b98a459 to
c503b34
Compare
c503b34 to
0cc6f96
Compare
marcinc
left a comment
There was a problem hiding this comment.
A few comments but generally loving the changes in this PR 💯
| return p.manifest | ||
| } | ||
|
|
||
| func WithComposeSources(c []string) Options { |
There was a problem hiding this comment.
A small potential improvement... We could use method chain on Project instead? then you could do something like Init().WithComposeSources(...).WithEvns(...).WithSkaffold(...).WithUI(...).
There was a problem hiding this comment.
@marcinc ... To clarify:
Do you mean instead of passing opts to Init(). And, creating extra methods on Project to perform the wiring?
But how would you know which method (.WithComposeSources(...) or .WithEvns) to call?
There was a problem hiding this comment.
yeh, that's what I meant. Current implementation is correct, just thought that method chain could be slightly nicer. To your question, I think you'd know which method to chain the same way you know it now.. an excertp:
return kev.InitProjectWithOptions(wd,
kev.WithComposeSources(files),
kev.WithEnvs(envs),
kev.WithSkaffold(skaffold))
# this would become
return kev.InitProject(wd).WithComposeSources(files).WithEnvs(envs).WithSkaffold(skaffold)There was a problem hiding this comment.
Tbh, I would rather leave it as is.
Otherwise, you would need to:
-> kev.InitProject(wd) to return a runner object.
-> Configure using With... methods.
... And you also then:
-> have to Run() the runner.
-> and do the other things that InitProjectWithOptions is doing for you.
There was a problem hiding this comment.
as said, it's good as is. If chaining brings extra complexity then that's fine.
pkg/kev/init.go
Outdated
| } | ||
| updateStep.Success(time.Second * 1) | ||
| case false: | ||
| createStep := sg.Add(fmt.Sprintf("Creating Skaffold config with deployment environments at: %s", skPath)) |
There was a problem hiding this comment.
with deployment environments at ... -> with deployment environment profiles at ...
847c810 to
da034ac
Compare
pkg/kev/init.go
Outdated
| } | ||
|
|
||
| s.Success(time.Second * 1) | ||
| s.Success(0) |
There was a problem hiding this comment.
makes sense for now I guess. Later we could have it managed centrally / or locally when needed by chaining .Wait(time.Seconds * 1) or similar. Could be done in a follow up PR as discussed.
Resolves https://github.com/appvia/kev-planning/issues/36
Please see ticket for proposed UI mockup.
To test this,
make.kev init. (The tests are broken but the binary works)Pending Items - COMPLETED