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

Issue 9698047: Have the verbose flag set the log level to INFO.

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by thumper
Modified:
10 years, 9 months ago
Reviewers:
dimitern, mp+167687, fwereade, rog
Visibility:
Public.

Description

Have the verbose flag set the log level to INFO. When the loggo branch landed yesterday, there was a change in behaviour with the --verbose flag, where it did log to stderr, but it was at the WARNING level, not INFO. https://code.launchpad.net/~thumper/juju-core/verbose-means-info/+merge/167687 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : Have the verbose flag set the log level to INFO. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -3 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/logging.go View 1 1 chunk +6 lines, -2 lines 1 comment Download
M cmd/logging_test.go View 2 chunks +23 lines, -1 line 0 comments Download

Messages

Total messages: 5
thumper
Please take a look.
10 years, 9 months ago (2013-06-06 03:59:55 UTC) #1
fwereade
LGTM
10 years, 9 months ago (2013-06-06 16:09:13 UTC) #2
dimitern
LGTM with one suggestion. https://codereview.appspot.com/9698047/diff/1/cmd/logging.go File cmd/logging.go (right): https://codereview.appspot.com/9698047/diff/1/cmd/logging.go#newcode56 cmd/logging.go:56: var level = loggo.INFO level ...
10 years, 9 months ago (2013-06-06 16:11:07 UTC) #3
thumper
*** Submitted: Have the verbose flag set the log level to INFO. When the loggo ...
10 years, 9 months ago (2013-06-07 01:06:07 UTC) #4
rog
10 years, 9 months ago (2013-06-07 08:38:44 UTC) #5
https://codereview.appspot.com/9698047/diff/5001/cmd/logging.go
File cmd/logging.go (right):

https://codereview.appspot.com/9698047/diff/5001/cmd/logging.go#newcode60
cmd/logging.go:60: logger := loggo.GetLogger("juju")
i'm not entirely sure it's right to restrict this to juju log messages only.
surely if i run juju bootstrap --debug, i'll want to see debug messages from
anything that's currently producing log messages (for example goose or maas)?

i'd change "juju" to "" here.
Sign in to reply to this message.

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