Code review comment for lp:~thumper/juju-core/make-syslog-port-configurable
Revision history for this message
| Tim Penhey (thumper) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
I'm not yet pushing up changes because I don't want to write more code
that is going to get argued over until we have a consensus on api/params
return types and the interface name.
https:/
File log/syslog/
https:/
log/syslog/
On 2013/11/21 05:17:42, axw wrote:
> fmt.Sprint(
Done.
https:/
File state/address.go (right):
https:/
state/address.
On 2013/11/22 00:46:52, wallyworld wrote:
> I think it's worth making this function return a struct.
> Since we are now returning a hodge podge of values, perhaps a better
method name
> is required
> eg DeployerAddressInfo or DeployerConnect
> I also think it may be worth naming this method to be the same as the
one on
> state, which is currently ServerAddresses().
Done.
https:/
state/address.
On 2013/11/21 05:17:42, axw wrote:
> Difficult to parse, IMHO. I'd prefer
> stateAddresses = appendPort(addrs, config.StatePort())
> apiAddresses = appendPort(addrs, config.APIPort())
> return stateAddresses, apiAddresses, config.
Now this returns a struct, it is more obvious.
https:/
File state/api/
https:/
state/api/
On 2013/11/21 05:17:42, axw wrote:
> I know I'm being pedantic, but a port isn't exactly an address.
Multiple API
> calls isn't great, though. It would be nice if we could make bulk
calls of an
> arbitrary sequence of API methods.
Changed struct name.
https:/
File worker/
https:/
worker/
On 2013/11/22 00:46:52, wallyworld wrote:
> On 2013/11/21 05:17:42, axw wrote:
> > This seems like the wrong place to be talking about the API. This
should just
> be
> > about domain objects, I think.
> +1
> params.
structs should
> escape the api infrastructure
OK, I disagree here on two fronts.
Firstly this interface should be renamed to have something to do with
the API. It defines the API methods that it calls.
Secondly there is already precedence for exporting api/params structures
as return parameters for client api method calls.
I don't see anything wrong with that as the api/params package is
defined to have no other dependencies, and is just a holding package for
structures. It seems stupid to redefine it just because.
This interface should be both renamed, and have the first two methods
removed, as they are no longer needed.
https:/