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

Issue 88800043: Remove legacy log functions.

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years ago by thumper
Modified:
10 years ago
Reviewers:
mp+216247, dave, jameinel
Visibility:
Public.

Description

Remove legacy log functions. This branch removes the old log.Errorf, log.Noticef, log.Infof, log.Debugf, and log.Warningf in favour of loggo functions. The rsyslog package is moved to be under utils, and the LoggedErrorf is moved to the errors package. A lot of very mechanical changes. https://code.launchpad.net/~thumper/juju-core/remove-old-log-functions/+merge/216247 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+184 lines, -316 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M charm/charm.go View 1 chunk +4 lines, -0 lines 0 comments Download
M charm/dir.go View 2 chunks +1 line, -3 lines 0 comments Download
M charm/dir_test.go View 1 chunk +1 line, -1 line 0 comments Download
M charm/repo.go View 4 chunks +3 lines, -4 lines 0 comments Download
M charm/repo_test.go View 2 chunks +4 lines, -4 lines 0 comments Download
M cmd/juju/plugin.go View 2 chunks +1 line, -2 lines 0 comments Download
M cmd/juju/publish.go View 5 chunks +5 lines, -6 lines 0 comments Download
M cmd/jujud/agent.go View 2 chunks +2 lines, -2 lines 0 comments Download
M container/kvm/container.go View 2 chunks +2 lines, -2 lines 0 comments Download
M container/kvm/kvm.go View 3 chunks +3 lines, -3 lines 0 comments Download
M downloader/downloader.go View 2 chunks +4 lines, -2 lines 0 comments Download
M environs/testing/tools.go View 2 chunks +1 line, -2 lines 0 comments Download
M errors/errors.go View 2 chunks +8 lines, -0 lines 0 comments Download
M juju/api.go View 2 chunks +0 lines, -4 lines 0 comments Download
M juju/conn.go View 4 chunks +7 lines, -4 lines 0 comments Download
D log/log.go View 1 chunk +0 lines, -51 lines 0 comments Download
D log/log_test.go View 1 chunk +0 lines, -84 lines 0 comments Download
M provider/common/state.go View 2 chunks +2 lines, -3 lines 0 comments Download
M rpc/rpc_test.go View 5 chunks +8 lines, -6 lines 0 comments Download
M state/api/watcher/watcher.go View 2 chunks +4 lines, -2 lines 0 comments Download
M state/apiserver/charmrevisionupdater/updater.go View 2 chunks +2 lines, -2 lines 0 comments Download
M state/apiserver/common/password.go View 3 chunks +6 lines, -3 lines 0 comments Download
M state/apiserver/common/resource.go View 2 chunks +1 line, -3 lines 0 comments Download
M state/presence/presence.go View 12 chunks +13 lines, -23 lines 1 comment Download
M state/watcher/watcher.go View 7 chunks +8 lines, -13 lines 1 comment Download
M store/lpad.go View 3 chunks +4 lines, -5 lines 0 comments Download
M store/server.go View 7 chunks +6 lines, -7 lines 0 comments Download
M store/store.go View 21 chunks +32 lines, -30 lines 0 comments Download
M testing/mgo.go View 5 chunks +6 lines, -7 lines 0 comments Download
M testing/testbase/log_test.go View 3 chunks +12 lines, -10 lines 0 comments Download
M utils/syslog/config_test.go View 1 chunk +2 lines, -2 lines 0 comments Download
M worker/authenticationworker/worker.go View 5 chunks +7 lines, -7 lines 0 comments Download
M worker/charmrevisionworker/revisionupdater.go View 2 chunks +4 lines, -2 lines 0 comments Download
M worker/cleaner/cleaner.go View 2 chunks +5 lines, -2 lines 0 comments Download
M worker/resumer/resumer.go View 2 chunks +4 lines, -3 lines 0 comments Download
M worker/rsyslog/rsyslog_test.go View 1 chunk +1 line, -1 line 0 comments Download
M worker/rsyslog/worker.go View 1 chunk +1 line, -1 line 0 comments Download
M worker/runner.go View 8 chunks +8 lines, -10 lines 0 comments Download

Messages

Total messages: 3
thumper
Please take a look.
10 years ago (2014-04-17 04:04:29 UTC) #1
dave_cheney.net
On 2014/04/17 04:04:29, thumper wrote: > Please take a look. LGTM
10 years ago (2014-04-17 04:08:34 UTC) #2
jameinel
10 years ago (2014-04-17 04:54:21 UTC) #3
LGTM though the functions that used the "debugf" wrapper should probably
actually be dropped to Tracef level.

https://codereview.appspot.com/88800043/diff/1/state/presence/presence.go
File state/presence/presence.go (right):

https://codereview.appspot.com/88800043/diff/1/state/presence/presence.go#new...
state/presence/presence.go:22: var logger =
loggo.GetLogger("juju.state.presence")
These were all disabled by default, should we actually reduce them down to
Tracef?

https://codereview.appspot.com/88800043/diff/1/state/watcher/watcher.go
File state/watcher/watcher.go (left):

https://codereview.appspot.com/88800043/diff/1/state/watcher/watcher.go#oldco...
state/watcher/watcher.go:279: debugf("state/watcher: got request: %#v", req)
Same here, all of these were actually lower-than-debug because you had to
explicitly enable the whole package to log.

We could make "juju.state.watcher" disabled by default, but I think just turning
all of these into Tracef makes the most sense.
Sign in to reply to this message.

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