Fix config reload in skaffold dev#2279
Conversation
dgageot
requested review from
balopat,
nkubala,
priyawadhwa and
tejal29
as code owners
dgageot
force-pushed
the
fix-2278
branch
2 times, most recently
from
d89cca8 to
6d0d63b
Compare
Fix GoogleContainerTools#2278 Signed-off-by: David Gageot <david@gageot.net>
Signed-off-by: David Gageot <david@gageot.net>
And only stop it at the end. Fix GoogleContainerTools#2281 Signed-off-by: David Gageot <david@gageot.net>
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
thanks, I think I misinterpreted what the dev loop was doing when the skaffold.yaml was changed.
do you want to just remove the build trigger here? since I'm working on some bigger changes that are going to change the way the build trigger is handled anyway, I'm not opposed to removing it until those changes are in
|
Isn’t it used? If it’s not, I’ll update the PR tomorrow to remove it. Or maybe we can merge this one and remove the trigger in another PR |
|
it will be used (eventually) if it's not right now, but it might not be yet. we should ask the IDE teams. I'm just not super happy with the current implementation but I suppose it's not doing a ton of harm leaving it in for now |
|
Let’s keep it here then and wait for you to change it |
Codecov Report
@@ Coverage Diff @@
## master #2279 +/- ##
==========================================
- Coverage 60.94% 60.93% -0.02%
==========================================
Files 186 186
Lines 7877 7871 -6
==========================================
- Hits 4801 4796 -5
- Misses 2690 2691 +1
+ Partials 386 384 -2
Continue to review full report at Codecov.
|
nkubala
left a comment
nkubala
left a comment
There was a problem hiding this comment.
alright, I'll address the build trigger stuff in a separate PR. this is fine for now, thanks for fixing it.
also, go to sleep 😪
4 participants
Fix #2278 and #2281
Signed-off-by: David Gageot david@gageot.net