jamfwereade: there you are09:02
jamI hereby revoke your ability to submit any more patches :)09:02
fwereadejam, hey dude, tyvm for all the review09:02
fwereadejam, haha09:03
fwereadejam, that's what happens when my family go away for a couple of days and there's noone on irc :)09:03
dimiternfwereade: you're on fire on friday :)09:04
fwereadedimitern, yeah, 'twas a great day09:04
fwereadejam, I was thinking of adding explicit tests for the charm test helpers to charm_test.go, would that be ok with you?09:05
jamfwereade: I've been told you don't write tests for test helpers, but *I'm* personally a fan of it.09:06
jam(Why is this test failing? Oh the test infrastructure is wrong)09:06
fwereadejam, yeah, me too, it's just been 4 years since I've been in an environment where someone else agrees with me on this09:06
dimiternjam: are we the only ones today for standup?10:26
jamdimitern: technically, but mgz is sitting on mumble10:33
fwereadedimitern, fwiw jam's observations on the txns are good ones, and I'm still trying to figure out the Right Thing to do in those cases, but I'm definitely not quite doing it right now11:47
fwereadedimitern, jam: the big question for me is what I can safely assume about ops that aren't constructed in plain view... *at the moment* we can simplify those my assuming how the op-creation methods work11:48
fwereadedimitern, jam: and *maybe* we should11:48
jamfwereade: I did a lot of reviewing, can you link for context?11:49
fwereadejam, https://codereview.appspot.com/8126044/diff/1/state/machine.go#newcode31011:50
fwereadejam, https://codereview.appspot.com/8126044/diff/1/state/machine.go#newcode44711:50
fwereadejam, I have ideas floating around about a layer that lets us work with ops without having to think so much11:52
jamfwereade: so if you have a case where retrying will succeed when it would otherwise fail, I'm happy to have the retries11:53
jamthough a more consistent "try twice" or "try three times" seems useful11:53
fwereadejam, "try twice" was deliberate, and I think it's correct but its form may be unhelpful -- the idea there is that we use the exact same gating checks across the board, but an aborted txn should guarantee that one of those will fail; we should therefore always return before attempting to execute a second txn11:56
fwereadejam, deserves a comment, I think11:57
fwereadejam, the reason for the "try three times" was because I expect lifecycle switches to be icky and demand retries, but neglected to observe that this one's simpler than I assumed11:58
fwereadejam, I would be most keen to go for a try-twice approach there for the above reasons11:58
fwereadejam, I'd like to roll up the duplicated bits of the txn-futzing code into something a little smarter, but I don't think I can justify it atm because it'd be at least half fishing-trip12:02
fwereademramm, hey, I think it's just me again -- do we need to meet? I have a pile of good reviews and I'll be landing them for the rest of the day I think, not much more to tell13:57
=== jcastro_ is now known as jcastro
mrammfwereade: if I have any questions about the board status, I'll let you know -- but no we don't need a meeting.13:58
fwereademramm, cool13:58
gary_postersorry for basic go question, but what is diff between []params.Port(nil) and []params.Port{} ?  Test expects first and obtains second14:50
gary_posterpointer to doc would be aok :-)14:52
dimiterngary_poster: I think the first one is just a nil slice, while the second one is an empty one. Both have len() == 0, but the first cannot be modified. The second is equivalent to make([]params.Port, 0)15:00
gary_posterah! thanks dimitern.  I'll see if I can switch one to the other...15:02
dimiterngary_poster: actually, both can be modified ok it seems: http://play.golang.org/p/Mr2Y49I2Q215:06
gary_posterdimitern, oh, weird.  So...should DeepEquals find them equivalent?15:06
gary_posterappend can return back a new instance IIRC15:07
gary_posterso you might still be right about initialization diffeerences15:07
dimiterngary_poster:  yeah, you're right - append creates a new slice15:08
dimiterngary_poster: deepequals should work I think15:08
dimiterngary_poster: they're of the same type15:08
gary_posterdimitern, http://pastebin.ubuntu.com/5667597/ ISTM that DeepEquals does not behave that way now?  Or am I missing something?15:09
dimiterngary_poster: https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/vNodYyoUu9Q - there are no docs about it, but there are some ML questions about it15:11
dimiterngary_poster: so "the value of an uninitialized slice is nil"15:12
fwereadegary_poster, I suggest using HasLen for slices that might be nil or empty15:13
fwereadegary_poster, assuming you're testing15:13
fwereadegary_poster, also works on maps15:13
dimiterngary_poster: HasLen 0 works on both maps and slices, even if nil, so should work15:14
gary_posterfwereade, ack, ty.  So given context (http://pastebin.ubuntu.com/5667597/) of testing larger struct, would you suggest refactoring that DeepEquals assertion to stepping over each attr individually?15:14
dimiterngary_poster: the issue for me here is why []T(nil) is returned when it need to return []T{} probably?15:14
gary_posteryeah, I was going to try and address that15:15
fwereadegary_poster, doing every field seems a bit nasty15:15
dimiterntry c.Assert(gotEntities[num].Port, DeepEquals, ent.Port)15:16
fwereadegary_poster, I'd probably prefer to fix either the return value or what the test checks for15:16
gary_posterfwereade, I thought so too.  Cool, I'll pursue that.  thank you again, fwereade and dimitern15:16
fwereadegary_poster, cheers15:16
dimiternif it fails, then I suppose digging into reflect.DeepEqual to see what might be the issue15:16
fwereadedimitern, AIUI nil is not the same as an empty slice, it just acts the same in most situations15:17
fwereadedimitern, DeepEquals is not one of those situations15:17
fwereadedimitern, consider maps -- nil maps act like empty maps in many situations, but not all15:18
fwereadedimitern, maps are a bit worse off because there's no equivalent of append for a map, so the difference is a little more obvious15:18
dimiternfwereade: yeah, I was thinking something in those lines15:22
dimiternfwereade: and the docs are sparse at places ("if it's not in the docs it's not allowed " etc.)15:22
fwereadegn all, hope to pop on and do a few of other people's reviews a bit later16:15
dimiternfwereade: have a good one16:15
=== Makyo is now known as Makyo|out
=== BradCrittenden is now known as bac__
=== bac__ is now known as bac
=== Makyo|out is now known as Makyo
gary_posterI'd really appreciate two reviews of this large but almost-but-not-entirely mechanical branch (circular imports): https://codereview.appspot.com/8086045/20:13
thumperhi gary_poster20:37
gary_posterhey thumper :-)20:37
thumpergary_poster: why the new package for params?20:39
thumpergary_poster: instead of using "series" for series and "name" for the service name with comments for "precise" and "wordpress", why not just structure the test so the results show "precise" and "wordpress" ?20:42
gary_posterthumper, I did not add the params package, I only added the values to the params package.  params package is supposed to be a place to put structs that are used both by state and by api20:43
gary_posterthe two are not to mix, AIUI; and certainly when they do you get circular imports20:44
thumperI bet most of those changes were mind-numbingly boring20:44
gary_posterwhy yes! :-)20:44
gary_posterthunper, +1 on changing the examples to the comments.20:45
gary_posterthumper, even20:45
* thumper writes it on the review20:46
* thumper looks at the kanban board for something to do20:50
m_3thumper: I don't have a leankit acct... would you mind linking lp:~mark-mims/+junk/pyju-packaging-with-alternatives to the packaging item21:27
m_3thumper: it's working, but martin can review/cleanup when he gets back from vaca21:28
thumperm_3: sure...21:28
m_3thumper: thanks!21:28
m_3thumper: go packaging should be able to do something similar21:29
* thumper nods21:29
m_3awesome... got an instance running with both 0.6 and 1.9.13 installed from packages23:25

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!