Code review comment for lp:~mbp/bzr/remove-logging

Revision history for this message
Robert Collins (lifeless) wrote :

You seem to be deleted some test coverage (the FormatDeprecationWarning test).

I like the concept of removing logging.

However, the changes for what goes to stdout/stderr are a bit mixed.

For reconcile, the actions taken were intended to be the output when I wrote it, I'm not entirely happy with that going to stderr.

For reconfigure I'm pretty sure Aaron would consider that the output of the command similarly.

For upgrade, the warning that a particular checkout won't be upgraded is something I consider part of the proper output of the command.

I guess is tied to my disagreeing with :
915 +We normally reserve stdout for bulk output data, such as diffs, logs, or
916 +bundles, and put all UIFactory output onto stderr.

I think that the *intended output* should go to stdout, and for some commands that is bulk data, for other commands its not.

As it stands I really am not comfortable with this landing, I think it will make our UI worse, for all that it does make a bunch of internal stuff cleaner.

review: Needs Information

« Back to merge proposal