Skip to content

Add --verbose flag and move some startup routine output to it.#9

Merged
febbraro merged 10 commits intodevelopfrom
task/verbose-flag
Mar 3, 2017
Merged

Add --verbose flag and move some startup routine output to it.#9
febbraro merged 10 commits intodevelopfrom
task/verbose-flag

Conversation

@grayside
Copy link
Contributor

This reduces the amount of noise produced by default when running rig start. If you want to see all the details, there is still rig --verbose start, or setting RIG_VERBOSE=1 as an environment variable. It might be I was too aggressive, worth reviewing.

One consideration, I'm thinking we might want a second level of verbosity within which to print out the commands ~ out.Learning.Println(cmd.Path, cmd.Args). That piece is captured in #5.

This fixes #4.

Copy link
Member

@febbraro febbraro left a comment

Choose a reason for hiding this comment

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

Some commands like rig status use stdout for all of their output. So currently, status outputs nothing. As part of this branch we can make status return "Running" or "Stopped" and verbose can output the whole table of data, if we want. We should evaluate the rest of the commands to determine if they all function with appropriate output when in a "info" setting vs. verbose.

@grayside
Copy link
Contributor Author

Hm, that's a bit tricky. The current change just switches the commands to output only if verbose is set, but does not address alternate processing if it isn't set. There are a couple different ways we could address this:

Option 1: Alter StreamCommand()

We add a second command parameter to StreamCommand to run if verbose mode is active, and let the caller construct different behaviors depending on the required verbosity. I don't like this as it limits the logic to the code we are executing.

StreamCommand(cmd *exec.Cmd, verboseCmd *exec.Cmd)

Option 2: Capture command output and return

Current StreamCommand runs the command and returns the error. If we switch to use cmd.Output() instead we can return the output or the error. This means that the caller can evaluate the current state of verbosity and process what to exec or output on it's own.

I think this one is better, but a bit more involved.

Grayside added 3 commits March 1, 2017 12:33
* origin/develop:
  Removed the need for cgo compilation flag
  Disambiguate docker-compose build from docker-compose run build.
  Fixed #7: Docker-based development of Rig.
@grayside
Copy link
Contributor Author

grayside commented Mar 1, 2017

Okay, pushed some changes.

  • Moved logging setup to a separate file.
  • Did some go fmt cleanup
  • Adds StreamCommandForce to sidestep the verbosity checking while still using the "exec API" layer we're trying to piece together.
  • status command checks verboseMode to decide which command to run.

Warning: log.New(os.Stdout, color.YellowString("[WARN] "), 0),
Error: log.New(os.Stderr, color.RedString("[ERROR] "), 0),
Verbose: log.New(verboseWriter, "[VERBOSE] ", 0),
IsVerbose: verbose,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

go fmt


func Logger() *RigLogger {
if logger == nil {
LoggerInit(false)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why passing in false here?

Copy link
Member

Choose a reason for hiding this comment

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

Well, I can;t imagine a case where the logger is nil, but just in case it is, we dont have access to the cli.Context to see if the GlobalFlag verbose is set. So, in lieu of information, lets use default behavior, which is not verbose. Does that make sense? I can also log a warning message about it. :)

@febbraro febbraro merged commit d91f5eb into develop Mar 3, 2017
@febbraro febbraro deleted the task/verbose-flag branch March 3, 2017 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create a global --verbose flag and use with rig start

2 participants