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

Issue 92440046: Log supercommand name.

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by cmars
Modified:
5 years, 9 months ago
Reviewers:
menn0, mp+220097, fwereade
Visibility:
Public.

Description

Log supercommand name. In SuperCommand.Run, log the supercommand's name instead of hard-coded 'juju'. https://code.launchpad.net/~cmars/juju-core/supercmd-logging/+merge/220097 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Log supercommand name. #

Total comments: 6

Patch Set 3 : Log supercommand name. #

Total comments: 2

Patch Set 4 : Log supercommand name. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+116 lines, -70 lines) Patch
A [revision details] View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/supercommand.go View 1 2 2 chunks +5 lines, -2 lines 0 comments Download
M cmd/supercommand_test.go View 1 2 3 3 chunks +43 lines, -10 lines 0 comments Download
M version/version.go View 1 2 2 chunks +3 lines, -0 lines 0 comments Download
M version/version_test.go View 1 2 5 chunks +63 lines, -58 lines 0 comments Download

Messages

Total messages: 13
cmars
Please take a look.
5 years, 9 months ago (2014-05-19 16:05:54 UTC) #1
menn0
LGTM apart from the suggestion made in-line. I'm ok with the change as it stands ...
5 years, 9 months ago (2014-05-19 23:23:44 UTC) #2
cmars
https://codereview.appspot.com/92440046/diff/1/cmd/supercommand.go File cmd/supercommand.go (right): https://codereview.appspot.com/92440046/diff/1/cmd/supercommand.go#newcode301 cmd/supercommand.go:301: logger.Infof("running %s-%s [%s]", c.Name, version.Current, runtime.Compiler) On 2014/05/19 23:23:44, ...
5 years, 9 months ago (2014-05-19 23:57:25 UTC) #3
menn0
On 2014/05/19 23:57:25, cmars wrote: > https://codereview.appspot.com/92440046/diff/1/cmd/supercommand.go > File cmd/supercommand.go (right): > > https://codereview.appspot.com/92440046/diff/1/cmd/supercommand.go#newcode301 > ...
5 years, 9 months ago (2014-05-20 00:08:50 UTC) #4
menn0
On 2014/05/20 00:08:50, menn0 wrote: > On 2014/05/19 23:57:25, cmars wrote: > > https://codereview.appspot.com/92440046/diff/1/cmd/supercommand.go > ...
5 years, 9 months ago (2014-05-20 00:12:59 UTC) #5
cmars
Please take a look.
5 years, 9 months ago (2014-05-20 04:17:55 UTC) #6
fwereade
I think we need to copy the value of runtime.Compiler into the version package, and ...
5 years, 9 months ago (2014-05-20 06:40:12 UTC) #7
cmars
Please take a look. https://codereview.appspot.com/92440046/diff/20001/cmd/supercommand.go File cmd/supercommand.go (right): https://codereview.appspot.com/92440046/diff/20001/cmd/supercommand.go#newcode304 cmd/supercommand.go:304: logger.Infof("running %s %s-%s [%s]", c.usagePrefix, ...
5 years, 9 months ago (2014-05-21 00:51:54 UTC) #8
cmars
5 years, 9 months ago (2014-05-21 00:52:12 UTC) #9
fwereade
LGTM. Really nice, thanks :). https://codereview.appspot.com/92440046/diff/40001/cmd/supercommand_test.go File cmd/supercommand_test.go (right): https://codereview.appspot.com/92440046/diff/40001/cmd/supercommand_test.go#newcode188 cmd/supercommand_test.go:188: version.Compiler = "llgo" Search ...
5 years, 9 months ago (2014-05-21 07:42:25 UTC) #10
cmars
Please take a look.
5 years, 9 months ago (2014-05-21 19:15:11 UTC) #11
cmars
Updated to use CleanupSuite.PatchValue. https://codereview.appspot.com/92440046/diff/40001/cmd/supercommand_test.go File cmd/supercommand_test.go (right): https://codereview.appspot.com/92440046/diff/40001/cmd/supercommand_test.go#newcode188 cmd/supercommand_test.go:188: version.Compiler = "llgo" On 2014/05/21 ...
5 years, 9 months ago (2014-05-21 19:16:31 UTC) #12
fwereade
5 years, 9 months ago (2014-05-21 20:04:06 UTC) #13
still LGTM
Sign in to reply to this message.

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