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

Issue 6116049: Move logging functionality into new cmd.Log type

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

Description

Move logging functionality into new cmd.Log type cmd.Log encapsulates everything that previously interacted with logging, and supplies InitFlagSet and Start methods, as used by SuperCommand when its Log field is non-nil. https://code.launchpad.net/~fwereade/juju/go-extract-cmd-log/+merge/103479 Requires: https://code.launchpad.net/~fwereade/juju/go-fix-cmd-help/+merge/103448 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 10

Patch Set 2 : Move logging functionality into new cmd.Log type #

Total comments: 10

Patch Set 3 : Move logging functionality into new cmd.Log type #

Patch Set 4 : Move logging functionality into new cmd.Log type #

Patch Set 5 : Move logging functionality into new cmd.Log type #

Unified diffs Side-by-side diffs Delta from patch set Stats (+188 lines, -198 lines) Patch
A [revision details] View 1 2 3 4 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/context.go View 1 2 3 4 2 chunks +0 lines, -22 lines 0 comments Download
M cmd/context_test.go View 1 2 3 4 3 chunks +3 lines, -50 lines 0 comments Download
M cmd/juju/main.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M cmd/jujud/main.go View 1 2 1 chunk +1 line, -1 line 0 comments Download
A cmd/logging.go View 1 2 1 chunk +46 lines, -0 lines 0 comments Download
A cmd/logging_test.go View 1 2 3 1 chunk +104 lines, -0 lines 0 comments Download
M cmd/supercommand.go View 1 2 3 4 3 chunks +11 lines, -19 lines 0 comments Download
M cmd/supercommand_test.go View 1 2 3 4 6 chunks +20 lines, -105 lines 0 comments Download

Messages

Total messages: 10
fwereade
Please take a look.
11 years, 11 months ago (2012-04-25 13:59:22 UTC) #1
rog
LGTM modulo trivia below. https://codereview.appspot.com/6116049/diff/1/cmd/logging.go File cmd/logging.go (right): https://codereview.appspot.com/6116049/diff/1/cmd/logging.go#newcode27 cmd/logging.go:27: // Start starts logging via ...
11 years, 11 months ago (2012-04-25 14:57:54 UTC) #2
fwereade
Please take a look. https://codereview.appspot.com/6116049/diff/1/cmd/logging.go File cmd/logging.go (right): https://codereview.appspot.com/6116049/diff/1/cmd/logging.go#newcode27 cmd/logging.go:27: // Start starts logging via ...
11 years, 11 months ago (2012-04-25 16:23:44 UTC) #3
fwereade
11 years, 11 months ago (2012-04-25 16:25:59 UTC) #4
niemeyer
Very nice refactoring. We'll need to sort the pre-req before this can go in. I ...
11 years, 11 months ago (2012-04-25 19:43:51 UTC) #5
fwereade
Please take a look. https://codereview.appspot.com/6116049/diff/5001/cmd/logging.go File cmd/logging.go (right): https://codereview.appspot.com/6116049/diff/5001/cmd/logging.go#newcode11 cmd/logging.go:11: // Log supplies the necessary ...
11 years, 11 months ago (2012-04-26 08:35:36 UTC) #6
fwereade
Please take a look.
11 years, 11 months ago (2012-04-26 08:46:29 UTC) #7
rog
On 2012/04/26 08:46:29, fwereade wrote: > Please take a look. LGTM
11 years, 11 months ago (2012-04-27 09:43:58 UTC) #8
niemeyer
Since IRC seems to be unstable for you today: <niemeyer> fwereade: I suspect the diff ...
11 years, 11 months ago (2012-04-27 14:13:08 UTC) #9
fwereade
11 years, 11 months ago (2012-04-27 15:01:54 UTC) #10
*** Submitted:

Move logging functionality into new cmd.Log type

cmd.Log encapsulates everything that previously interacted with logging, and
supplies InitFlagSet and Start methods, as used by SuperCommand when its Log
field is non-nil.

R=rog, niemeyer
CC=
https://codereview.appspot.com/6116049
Sign in to reply to this message.

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