Skip to content
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

Experimental UI mode for skaffold dev #1533

Merged
merged 2 commits into from
Feb 5, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Jan 25, 2019

Please have a try and tell me what you think about it!

@codecov-io
Copy link

codecov-io commented Jan 25, 2019

Codecov Report

Merging #1533 into master will decrease coverage by 0.61%.
The diff coverage is 8.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1533      +/-   ##
==========================================
- Coverage   46.88%   46.27%   -0.62%     
==========================================
  Files         118      118              
  Lines        5042     5109      +67     
==========================================
  Hits         2364     2364              
- Misses       2446     2512      +66     
- Partials      232      233       +1
Impacted Files Coverage Δ
pkg/skaffold/config/options.go 88.23% <ø> (ø) ⬆️
cmd/skaffold/app/cmd/dev.go 0% <0%> (ø) ⬆️
pkg/skaffold/color/formatter.go 72.72% <0%> (-7.28%) ⬇️
pkg/skaffold/kubernetes/log.go 6.55% <0%> (ø) ⬆️
pkg/skaffold/runner/dev.go 56.52% <87.5%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8efe309...dcfb2e3. Read the comment docs.

@dgageot dgageot force-pushed the ui-mode branch 6 times, most recently from 81b0239 to 940ff12 Compare January 29, 2019 05:39
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the visual separation a lot and it gives a foundation for a "dashboard" kind of view that we can implement iteratively on top of this direction. Has a lot of potential.

I really don't like that I can't scroll up - I'm losing history basically.
I think if we figure out scrolling we can merge this.
I would also like to see a "legend" describing "Ctrl + C can exit" or similar.

@dgageot
Copy link
Contributor Author

dgageot commented Jan 30, 2019

@balopat I'll try to see how this can be improved. I was thinking we could merge this code if an experimental flag can be used to activate this UI. wdyt?

@dgageot dgageot requested a review from priyawadhwa as a code owner January 30, 2019 08:50
@dgageot dgageot force-pushed the ui-mode branch 5 times, most recently from e8f792d to b4993ba Compare February 1, 2019 05:55
@balopat
Copy link
Contributor

balopat commented Feb 1, 2019

I'm okay having behind an experimental flag!

@dgageot
Copy link
Contributor Author

dgageot commented Feb 2, 2019

@balopat should be good to review then

@dgageot dgageot force-pushed the ui-mode branch 4 times, most recently from ba6a88d to 5a8bdcf Compare February 5, 2019 09:14
Only for `skaffold dev`

Signed-off-by: David Gageot <david@gageot.net>
Only for `skaffold dev`

Signed-off-by: David Gageot <david@gageot.net>
@dgageot dgageot merged commit 09ee966 into GoogleContainerTools:master Feb 5, 2019
@tjerkw
Copy link

tjerkw commented Feb 20, 2019

This feature is broken for me:

skaffold dev --experimental-gui
FATA[0034] exiting dev mode because first run failed: build failed: building [quay.io/tiqets/tiqetsweb-solr]: build artifact: running build: signal: broken pipe

I'm also expected a proper GUI. But this is just a text-interface :p

@josefkorbel
Copy link

I really like it to be honest, good work! Having problems only with scrolling (history loss) as mentioned before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants