Merge lp://staging/~thumper/juju-core/verbose-means-info into lp://staging/~juju/juju-core/trunk

Reviewers: mp+167687_code.launchpad.net,

Message:
Please take a look.

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)

Please review this at https://codereview.appspot.com/9698047/

Affected files:
   A [revision details]
   M cmd/logging.go
   M cmd/logging_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: cmd/logging.go
=== modified file 'cmd/logging.go'
--- cmd/logging.go 2013-06-05 02:44:14 +0000
+++ cmd/logging.go 2013-06-06 03:55:42 +0000
@@ -52,9 +52,13 @@
   } else {
    loggo.RemoveWriter("default")
   }
- if l.Debug {
+ if l.Verbose || l.Debug {
+ var level = loggo.INFO
+ if l.Debug {
+ level = loggo.DEBUG
+ }
    logger := loggo.GetLogger("juju")
- logger.SetLogLevel(loggo.DEBUG)
+ logger.SetLogLevel(level)
   }
   loggo.ConfigureLogging(l.Config)
   return nil

Index: cmd/logging_test.go
=== modified file 'cmd/logging_test.go'
--- cmd/logging_test.go 2013-05-30 03:41:19 +0000
+++ cmd/logging_test.go 2013-06-06 03:55:22 +0000
@@ -5,11 +5,13 @@

  import (
   "io/ioutil"
+ "path/filepath"
+
   . "launchpad.net/gocheck"
   "launchpad.net/juju-core/cmd"
   "launchpad.net/juju-core/log"
   "launchpad.net/juju-core/testing"
- "path/filepath"
+ "launchpad.net/loggo"
  )

  type LogSuite struct {
@@ -38,6 +40,26 @@
   c.Assert(l.Config, Equals, "juju.cmd=INFO;juju.worker.deployer=DEBUG")
  }

+func (s *LogSuite) TestVerboseSetsLogLevel(c *C) {
+ l := &cmd.Log{Verbose: true}
+ ctx := testing.Context(c)
+ err := l.Start(ctx)
+ c.Assert(err, IsNil)
+
+ logger := loggo.GetLogger("juju")
+ c.Assert(logger.GetLogLevel(), Equals, loggo.INFO)
+}
+
+func (s *LogSuite) TestDebugSetsLogLevel(c *C) {
+ l := &cmd.Log{Debug: true}
+ ctx := testing.Context(c)
+ err := l.Start(ctx)
+ c.Assert(err, IsNil)
+
+ logger := loggo.GetLogger("juju")
+ c.Assert(logger.GetLogLevel(), Equals, loggo.DEBUG)
+}
+
  func (s *LogSuite) TestStderr(c *C) {
   l := &cmd.Log{Verbose: true, Config: "<root>=INFO"}
   ctx := testing.Context(c)

LGTM

https://codereview.appspot.com/9698047/

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 := loggo.INFO

https://codereview.appspot.com/9698047/

*** Submitted:

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.

R=fwereade, dimitern
CC=
https://codereview.appspot.com/9698047

https://codereview.appspot.com/9698047/

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.

https://codereview.appspot.com/9698047/