Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(203)

Issue 8674043: deuglify logging for main packages. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 11 months ago by dimitern
Modified:
10 years, 11 months ago
Reviewers:
mp+158445
Visibility:
Public.

Description

deuglify logging for main packages. Removed "cmd/juju*: " badge from all commands. Also remove the "JUJU:" prefix and the automatic command prefix and name which was just repeating what's already in the message itself. So now, instead of: ERROR JUJU:jujutest:blah jujutest blah command failed: BAM! ERROR JUJU:juju:bootstrap juju bootstrap command failed: dummy.Bootstrap is broken INFO JUJU:test hello We'll have just: ERROR command failed: BAM! INFO hello ERROR command failed: dummy.Bootstrap is broken The timestamp with format like 2013/04/12 09:59:16 is still there, before the message. https://code.launchpad.net/~dimitern/juju-core/032-deuglify-logging-for-main-packages/+merge/158445 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : deuglify logging for main packages. #

Total comments: 3

Patch Set 3 : deuglify logging for main packages. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -60 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/cmd.go View 1 2 1 chunk +1 line, -1 line 0 comments Download
M cmd/juju/main_test.go View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M cmd/juju/publish.go View 4 chunks +5 lines, -5 lines 0 comments Download
M cmd/juju/ssh.go View 1 chunk +2 lines, -2 lines 0 comments Download
M cmd/juju/synctools.go View 6 chunks +6 lines, -6 lines 0 comments Download
M cmd/jujud/agent.go View 3 chunks +7 lines, -7 lines 0 comments Download
M cmd/jujud/machine.go View 6 chunks +6 lines, -6 lines 0 comments Download
cmd/jujud/unit.go View 1 chunk +1 line, -1 line 0 comments Download
M cmd/jujud/upgrade.go View 6 chunks +11 lines, -11 lines 0 comments Download
M cmd/logging.go View 1 3 chunks +1 line, -8 lines 0 comments Download
M cmd/logging_test.go View 1 4 chunks +7 lines, -7 lines 0 comments Download
M cmd/supercommand.go View 1 1 chunk +0 lines, -3 lines 0 comments Download
M cmd/supercommand_test.go View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7
dimitern
Please take a look.
10 years, 11 months ago (2013-04-11 17:53:00 UTC) #1
rog
On 2013/04/11 17:53:00, dimitern wrote: > Please take a look. LGTM though weren't you going ...
10 years, 11 months ago (2013-04-11 18:06:03 UTC) #2
dfc
LGTM. Thank you.
10 years, 11 months ago (2013-04-12 05:15:00 UTC) #3
dimitern
On 2013/04/11 18:06:03, rog wrote: > On 2013/04/11 17:53:00, dimitern wrote: > > Please take ...
10 years, 11 months ago (2013-04-12 08:11:33 UTC) #4
dimitern
Please take a look.
10 years, 11 months ago (2013-04-12 08:21:32 UTC) #5
rog
nice. LGTM with one small consideration below. https://codereview.appspot.com/8674043/diff/6001/cmd/logging.go File cmd/logging.go (left): https://codereview.appspot.com/8674043/diff/6001/cmd/logging.go#oldcode35 cmd/logging.go:35: return l.Logger.Output(calldepth, ...
10 years, 11 months ago (2013-04-12 08:27:52 UTC) #6
dimitern
10 years, 11 months ago (2013-04-12 08:53:21 UTC) #7
*** Submitted:

deuglify logging for main packages.

Removed "cmd/juju*: " badge from all commands.

Also remove the "JUJU:" prefix and the automatic
command prefix and name which was just repeating
what's already in the message itself.
So now, instead of:

ERROR JUJU:jujutest:blah jujutest blah command failed: BAM!
ERROR JUJU:juju:bootstrap juju bootstrap command failed: dummy.Bootstrap is
broken
INFO JUJU:test hello

We'll have just:
ERROR command failed: BAM!
INFO hello
ERROR command failed: dummy.Bootstrap is broken

The timestamp with format like 2013/04/12 09:59:16
is still there, before the message.

R=rog, dfc
CC=
https://codereview.appspot.com/8674043

https://codereview.appspot.com/8674043/diff/6001/cmd/supercommand_test.go
File cmd/supercommand_test.go (right):

https://codereview.appspot.com/8674043/diff/6001/cmd/supercommand_test.go#new...
cmd/supercommand_test.go:112: c.Assert(bufferString(ctx.Stderr), Matches, `^.*
ERROR jujutest blah command failed: BAM!
On 2013/04/12 08:27:52, rog wrote:
> i wonder if "command" is necessary here.
> 
> ERROR jujutest blah failed: BAM!
> 
> would perhaps be better i think.

As agreed online, I'll leave just "ERROR command failed: BAM!"
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b