Simplify doDev() #2815
Simplify doDev() #2815
Conversation
dgageot
requested review from
balopat,
nkubala,
priyawadhwa and
tejal29
as code owners
dgageot
force-pushed
the
simplify-dodev
branch
from
d7b7121 to
b22f2e2
Compare
Codecov Report
|
Signed-off-by: David Gageot <[email protected]>
dgageot
force-pushed
the
simplify-dodev
branch
from
b22f2e2 to
efe903b
Compare
| @@ -34,30 +34,30 @@ import ( | |||
| var ErrorConfigurationChanged = errors.New("configuration changed") | |||
|
|
|||
| func (r *SkaffoldRunner) doDev(ctx context.Context, out io.Writer) error { | |||
| actionPerformed := false | |||
| if r.changeSet.needsReload { | |||
| return ErrorConfigurationChanged | |||
There was a problem hiding this comment.
I think we want the logs muted during this time right? otherwise we'll have deployment logs interleaved with the build/deploy logs from the new skaffold runner.
also, r.changeSet.resetReload()?
There was a problem hiding this comment.
In fact, the logs will be stopped when this function returns: here.
Also, a new runner, with a new changeset will be created.
There was a problem hiding this comment.
We should write tests expressing these expectations.
It seems that there was no log muting in the current version - but I think I agree that interleaving might happen - are you suggesting adding log muting?
There was a problem hiding this comment.
ahh very nice, thanks for clarifying. this is much cleaner! 👍
Signed-off-by: David Gageot [email protected]