-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix config reload in skaffold dev #2279
Conversation
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>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 😪
Fix #2278 and #2281
Signed-off-by: David Gageot david@gageot.net