phase2 / rig

Outrigger command line tool
MIT License
11 stars 8 forks source link

Fixed prune with a confirmation, a force, and removed the spinner #145

Closed febbraro closed 6 years ago

febbraro commented 6 years ago

@grayside @tekante I added the Warn and the spinner. The spinner worked for me, kinda, can you test it to see if it is something we would want to use?

febbraro commented 6 years ago

screenshot 2018-02-22 15 27 45

tekante commented 6 years ago

It worked for me though we still seem to have some weird conditions between when the spinner gets overwritten versus when it is augmented. See attached screenshot:

rig-prune

I don't know that those would be a big enough blocker to stop proceeding but it would be nice to figure out what is going on there (I think @grayside dug a good bit and this may be the best we can do).

I'll also note that I had to update to go 1.10 to build it as nfpm now depends on a Format element in archive/tar that 1.9 doesn't seem to recognize.

febbraro commented 6 years ago

@grayside Do you have any thoughts here? I'm torn if the intermittent appending vs newlines (see "Deleted Containers/Images" vs "Deleted Volumes") . Do we care about these inconsistencies? Is it worth it for the spinner? or should we disable the spinner and just drop a quick message like "This may take a while...."?

grayside commented 6 years ago

How much wait time is there when nothing is being output to the screen? If it takes awhile but doesn't leave the user waiting to see progress, I think we should skip the spinner.

I find it jarring to mix the spinner with regular logs in the output for a single command execution, but prune is not a command we are ever likely to mix with something else, and if we do we can address it then.