[21:15] morning all [21:19] simple review if anyone's interested: https://github.com/juju/juju/pull/91 [21:33] waigani: mornin'. looking at that PR now. [21:33] menn0: sweet, thanks. I suspect I may have to add more tests - just started with the basic one. [21:39] waigani: is the description for the PR correct? You say this implements the client side of the UserInfo but this looks to be the server side. [21:40] waigani: ignore me... [21:40] waigani: I'm tired. You're right. [21:41] menn0: hehe, I was just composing my explanation [21:41] I don't particularly like the "todo, not tested" bits [21:41] I got my directory paths and file names mixed up [21:41] can we not work out how to test it? [21:42] that was going to be my first review comment... you should be able to mock out the API call to test the error case [21:42] thumper: okay, there are other places in the API with the same todo - I was being lazy and followed suit [21:48] waigani, thumper: I've just commented with a suggestion on how to do this. [21:50] menn0: yay thanks. I'll plow through these emails asap and get onto it. [21:56] waigani: I've added a few more comments and I'm done for now. [21:56] menn0: Great. Thank you! [22:34] argh... [22:34] my head hurts [22:34] although not boxing this time [22:34] lots of thinking around identities and users [22:39] thumper: and lack of wine? [22:39] not yet [22:39] had a drink last night [22:39] but it will be the last for a while [22:39] the rest of us will have to drink more to put up with you! [22:40] I'm not sure how to mock out a method, without creating a constructor for the object/struct [22:41] oh, haha - I just spotted NewUserManagerAPI, looks like a constructor [22:41] ignore me [22:56] thumper, menn0: I've made the server side UserInfo mockable. How do I mock it out from the client side? It is called over the wire and initialized separately from the client. [22:56] waigani: you shouldn't do it that way [22:56] oh [22:56] you should mock out the client's user manager [22:56] not the server side [22:57] back to the drawing board [22:57] at least I think... [22:57] are there examples of this anywhere? [22:57] I have vague recollections of this before [22:58] thumper: i get what you're saying, mock out the server endpoint on the client side [22:58] yeah [22:58] I'll give it another go after standup [22:59] api/client_test.go line 75 [22:59] thumper: sweet, thanks [22:59] waigani, thumper: in this case it's really easy isn't it? All calls are made through a single standalone function (call()) so patch that function for tests where you'd like to create an error condition. [22:59] not sure how helpful it'll be [22:59] menn0: that seems the obvious place... === psivaa is now known as psivaa-afk [23:20] connection dropped, we were done right? [23:22] Any feedback from Juju documenters? https://lists.ubuntu.com/archives/juju-dev/2014-June/002708.html [23:23] waigani: yep, we were done [23:45] JoshStrobl: we agree it needs to be updated :-) [23:49] bikeshedding required! when a relation hook fails should the agent info in the status output look like: [23:49] a) hook failed: some-relation-changed for mysql:server [23:49] b) hook failed: some-relation-changed with mysql:server [23:49] c) hook failed: some-relation-changed for wordpress:mysql to mysql:server [23:49] (if the error was being reported for the wordpress charm) [23:50] currently we get something like: hook failed: some-relation-changed [23:50] c [23:50] menn0: call is a method of Client struct. If I was to make the method mockable, I'd have to add a constructor to the struct. [23:51] waigani: I'm talking about the call function at usermanager/client.go:21 [23:52] menn0: oooohhh [23:52] thumper: any thoughts on the colour of the bike shed? [23:55] waigani: to patch you'll probably need to introduce another level of indirection (i.e. a standalone function which takes st and the other args that call() takes. [23:56] menn0: yep, I just missed the call func in usermanager - I went looking further up the tree in api/client [23:58] davecheney: any preference on how relation hook failures should be displayed in status output?