|
|
|
utils/syslog: log to all state machines
https://code.launchpad.net/~wwitzel3/juju-core/008-ha-rsyslog/+merge/217208
(do not edit description out of merge proposal)
Patch Set 1 #Patch Set 2 : utils/syslog: log to all state machines #
Total comments: 1
Total messages: 5
|
wwitzel
Please take a look.
|
11 years, 8 months ago (2014年05月09日 13:10:57 UTC) #1 | ||||||||||||||||||||||||||||||
Please take a look.
Please take a look.
Keep in mind that I will working with Curtis to get tests for this in to the CI build.
LGTM I guess. I don't know the rsyslog format, though, so can't really critique it. I understand the changes and they look sane as far as that is concerned. https://codereview.appspot.com/96190043/diff/20001/utils/syslog/config.go File utils/syslog/config.go (right): https://codereview.appspot.com/96190043/diff/20001/utils/syslog/config.go#new... utils/syslog/config.go:213: var stateServerHosts = func() []string { note that you can make this stateServerHosts := func() []string { and drop the var. Doesn't need to be fixed, just saying for future reference.
On 2014年05月12日 15:24:16, nate.finch wrote: > LGTM I guess. I don't know the rsyslog format, though, so can't really critique > it. I understand the changes and they look sane as far as that is concerned. > Tested working in non-HA and HA mode with EC2, MAAS, and Canonistack. > https://codereview.appspot.com/96190043/diff/20001/utils/syslog/config.go > File utils/syslog/config.go (right): > > https://codereview.appspot.com/96190043/diff/20001/utils/syslog/config.go#new... > utils/syslog/config.go:213: var stateServerHosts = func() []string { > note that you can make this stateServerHosts := func() []string { and drop the > var. Doesn't need to be fixed, just saying for future reference. Right. Makes sense.