Skip to content

Conversation

@parkouss
Copy link
Contributor

No description provided.

Add a -p/--process-output option to show or hide process output
logging. Process output logging is no more controlled by the debug flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like process output logging is not entirely clear. I would at least say Manage process output (stdout) logging.

Also, why isn't this a store_true argument? I don't see why specifying 1 or 0 on the command line should really be necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically to allow the user to enable this option in the config file, but still allowing him to disable it from the command line.

store_true is not something that can be easily overridden from the command line when you have it defined in the config file. it would require a --no-output to deactivate it, and I don't see any simple way to manage it with argparse. I tend to use now argument that always take a parameter, so we can override the value easily when it is defined in the config file.
Make sense ? If you have another idea, tell me. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you make the option store_true or store_false depending on what the config file says?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. That would be weird to me, cmdline options depending on the config file... What if I just replace "1" and "0" possible values (yeah, that is quite horrible now that I see that again) to "stdout" and "none" ? There would be some space in case we want to add another new action, and "-P=stout" looks ok to me.
Will ?

@parkouss
Copy link
Contributor Author

Closed because the code is in #353.

@parkouss parkouss closed this Nov 20, 2015
@parkouss parkouss deleted the process-logging branch November 20, 2015 10:31
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.

2 participants