| anastasiamac | ericsnow: thnx for shipit :) | 00:00 |
|---|---|---|
| ericsnow | anastasiamac: I figured it would avoid any confusion :) | 00:00 |
| ericsnow | anastasiamac: did you get your dependencies.tsv stuff sorted out? | 00:01 |
| anastasiamac | ericsnow: it's gr8ly appreciated! i was worried how it horrible it'll look to merge without shiptit! but m loving the echo ;) | 00:01 |
| ericsnow | anastasiamac: :) | 00:01 |
| anastasiamac | ericsnow: yes! ur advice was brilliant | 00:01 |
| ericsnow | anastasiamac: good; I was just echoing what others had shown me :) | 00:02 |
| anastasiamac | ericsnow: :) | 00:03 |
| davecheney | thumper ping | 00:07 |
| jw4 | axw: good point about _ubuntu* : I advocated for changing the name from _nonlinux, but your point about conflating usage and what is actually compiled is good. | 01:01 |
| jw4 | axw: would you keep the name _nonlinux? | 01:01 |
| axw | jw4: *shrug* I don't see a reason to change it. why did you want to change it away from that? | 01:02 |
| jw4 | axw: to me that was confusing because it looked like _linux, _darwin, _windows etc. and made me question whether the _non... was a valid build constraint | 01:02 |
| axw | I see | 01:02 |
| jw4 | axw: I'm fine with leaving the name, but I would love to see a less ambiguous name | 01:03 |
| axw | I think that if someone uses that incorrectly it'll become obvious pretty quick, since having multiple build tags almost always involves having a common entry point | 01:03 |
| axw | which would cause an error if the tags actually resolve | 01:04 |
| jw4 | kk, that's reasonable to me | 01:04 |
| ericsnow | axw, jw4: thanks for taking a look | 01:06 |
| axw | nps | 01:07 |
| ericsnow | axw, jw4: that change doesn't pay enough so I've dropped it :) | 01:07 |
| jw4 | ericsnow: cool - thanks for considering it :) | 01:07 |
| ericsnow | jw4: the comment in there should suffice | 01:08 |
| jw4 | yep | 01:09 |
| axw | wallyworld: I've lost my webcam, trying to fix it now... | 01:16 |
| wallyworld | np | 01:16 |
| davecheney | the HUD woke up to tell me I had 10% battery life, then to prove it's point went to 100% cpu and drained my battery | 01:31 |
| mattyw | wallyworld, quick question about juju/names? | 04:17 |
| wallyworld | sure | 04:22 |
| wallyworld | mattyw: ? | 04:28 |
| mattyw | wallyworld, I reckon UnitService (https://github.com/juju/names/blob/master/unit.go#L54) should return an error rather than panic | 04:28 |
| mattyw | wallyworld, it appears there are number of places in core that don't check IsValidUnit before making this call | 04:29 |
| mattyw | wallyworld, I was going to make the change so that UnitService returned (string, error) but thought I'd check with someone first | 04:29 |
| wallyworld | mattyw: sorry, my connection is a bit flaky. what was your question? | 04:35 |
| mattyw | wallyworld, does that sound like a reasonable change - UnitService not panicing - but returning an error if the unit name is invalid | 04:36 |
| wallyworld | mattyw: i'd be reluctant to change just that one method when all others panic if the name is invalid. i think the expecation is that names are validated before calling into the package | 04:39 |
| === kadams54_ is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| wallyworld | axw_: got time for a hangout? | 05:23 |
| axw_ | wallyworld: sure, I need to duck down in 10m to get something out of the oven tho | 05:25 |
| axw_ | wallyworld: see you in 1:1 | 05:25 |
| wallyworld | ok, let's make it quick | 05:25 |
| axw_ | wallyworld: need to write a bunch of tests, but I should be able to propose a branch soon that connects "juju machine add --disks" to the machine provisioner | 05:38 |
| axw_ | I just got ec2 to provision a machine with extra volumes once again | 05:38 |
| === axw_ is now known as axw | ||
| wallyworld | axw: that is \o/ | 05:56 |
| anastasiamac | wallyworld: thnx for shipit! | 06:28 |
| wallyworld | np :-) | 06:28 |
| anastasiamac | wallyworld: it's my fastest yet :) | 06:28 |
| wallyworld | indeed | 06:29 |
| === liam_ is now known as Guest14772 | ||
| dimitern | niemeyer, hey, if you have some time I'd appreciate a review on https://github.com/go-amz/amz/pull/12 please | 11:46 |
| anastasiamac | voidspace: ping | 12:21 |
| voidspace | anastasiamac: poing | 12:21 |
| voidspace | *pong even | 12:21 |
| perrito666 | morning everyone | 12:21 |
| voidspace | perrito666: morning | 12:21 |
| anastasiamac | perrito666: morning :) | 12:21 |
| voidspace | ah, I'm OCR today | 12:21 |
| voidspace | forgot | 12:21 |
| anastasiamac | voidspace: r u ocr? | 12:21 |
| voidspace | *damn* :-) | 12:21 |
| voidspace | anastasiamac: I believe so... | 12:21 |
| anastasiamac | voidspace: thnx ;) | 12:21 |
| anastasiamac | voidspace: could u plz have a look at http://reviews.vapour.ws/r/701/ | 12:21 |
| anastasiamac | voidspace: it's tiny and i'd love to land it in 1.22 | 12:22 |
| anastasiamac | voidspace: well, for 1.22 | 12:22 |
| voidspace | anastasiamac: looking now | 12:22 |
| anastasiamac | voidspace: greatly appreciated!! | 12:23 |
| voidspace | anastasiamac: looks like a nice straightforward improvement | 12:40 |
| voidspace | anastasiamac: LGTM | 12:40 |
| perrito666 | when creating a new facade version for uniter, should the new one embed the previous one? | 12:40 |
| anastasiamac | voidspace: thnx :) | 12:41 |
| voidspace | dimitern: I guess landing an mp for gomaasapi is a manual merge | 12:45 |
| dimitern | voidspace, it is I think | 12:45 |
| voidspace | done | 12:46 |
| dimitern | voidspace, \o/ | 12:50 |
| voidspace | dimitern: as far as I can tell it's not possible to go from node -> nodegroup via the maas api | 12:55 |
| voidspace | dimitern: I'm just double checking the details api to see if it's in that | 12:56 |
| voidspace | node id -> nodegroup uuid | 12:57 |
| voidspace | and details is xml, yummy | 13:02 |
| voidspace | and doesn't include nodegroup | 13:03 |
| dimitern | voidspace, right, that sounds like another bug worth reporting :) | 13:04 |
| voidspace | yeah | 13:04 |
| voidspace | I think they're getting sick of me... | 13:04 |
| dimitern | :D | 13:05 |
| dimitern | it's unavoidable though.. | 13:05 |
| anastasiamac | voidspace: 've changed the code after ur lgtm | 13:32 |
| anastasiamac | voidspace: could u plz cast ur eyes over it again? | 13:32 |
| anastasiamac | voidspace: changed the facade and its test to reflect the fact the each entity in the list could have its annotations... | 13:33 |
| anastasiamac | voidspace: should be a tivial modiifcation too | 13:33 |
| anastasiamac | voidspace: modification even :) | 13:33 |
| === kadams54 is now known as kadams54-away | ||
| niemeyer | dimitern: Review is up | 14:18 |
| dimitern | niemeyer, thanks! I'm making the suggested changes now | 14:19 |
| niemeyer | dimitern: np! | 14:20 |
| dimitern | niemeyer, how do you feel about renaming v2-dev to v2? | 14:29 |
| dimitern | so we can start using it in juju | 14:29 |
| anastasiamac | sinzui: o/ | 14:29 |
| sinzui | hi anastasiamac | 14:30 |
| anastasiamac | sinzui: m trying to land something for 1.22 but keep getting "No space left on device" | 14:30 |
| anastasiamac | sinzui: what shall i do? | 14:30 |
| sinzui | I will invesitigate | 14:30 |
| anastasiamac | sinzui: thnx :) | 14:31 |
| anastasiamac | sinzui: also it's past midnite here but I really want it to land... when r u plannning to take 1.22? (in how many hrs?) :P | 14:32 |
| sinzui | the juju-core-slave has 23G of space. I think the ec2 instance is low on space | 14:32 |
| sinzui | anastasiamac, we cannot take 1.22 until 1.21.0 is created. the feature freeze is artifical | 14:33 |
| voidspace | anastasiamac: sure | 14:33 |
| sinzui | anastasiamac, the ec2 image had 6.5G of free space when the test started | 14:34 |
| sinzui | anastasiamac, is this failure repeatable? | 14:35 |
| anastasiamac | voidspace: thnx ;-) | 14:36 |
| anastasiamac | sinzui: i've tried to merge twice and got it both times ;) | 14:37 |
| voidspace | anastasiamac: looking at the changes | 14:37 |
| sinzui | mgz, have you seen cases where the ec2 instance starts with 7.75G free, then the tests fail with no space left on device | 14:37 |
| voidspace | anastasiamac: those changes look good to me | 14:38 |
| voidspace | updated ShipIt | 14:38 |
| anastasiamac | voidspace: tyvm :) | 14:38 |
| TheMue | sinzui: when will the 1.22 branch be created? | 14:38 |
| sinzui | TheMue, I may create it at my EOD or tomorrow because of CI test lag. I will select the last passing rev | 14:40 |
| TheMue | sinzui: ok, we hope to get one more action PR into it until then, thx | 14:41 |
| wwitzel3 | ericsnow: ping | 14:51 |
| ericsnow | wwitzel3: pong | 14:57 |
| wwitzel3 | ericsnow: hi :) | 14:57 |
| bloodearnest | #join python-uk | 14:59 |
| bloodearnest | #/oin python-uk | 14:59 |
| bloodearnest | whoops | 14:59 |
| ericsnow | wwitzel3: you saw my commits (formatting and tests)? | 15:00 |
| wwitzel3 | ericsnow: I did | 15:00 |
| bodie_ | CI says "Can't take a write lock while out of disk space" | 15:01 |
| bodie_ | resolution = ? | 15:01 |
| anastasiamac | sinzui: tried to merge 3rd time, got the same "No space left on device" | 15:01 |
| sinzui | anastasiamac, :/ | 15:01 |
| sinzui | anastasiamac, as this is ec2 I feel pretty helpless | 15:01 |
| anastasiamac | sinzui: it's getting l8 for me. I'll try to land it in the morning - about 4 or 5 hrs from now :) | 15:01 |
| wwitzel3 | ericsnow: yeah, looks good | 15:02 |
| ericsnow | wwitzel3, perrito666: standup? | 15:02 |
| ericsnow | wwitzel3: oh, good :) | 15:02 |
| sinzui | anastasiamac, I have a clue | 15:03 |
| anastasiamac | sinzui: k :) | 15:04 |
| anastasiamac | sinzui: m mean I m sure u do :) | 15:04 |
| sinzui | I think the git-merge-juju job explicitly creates a 2G tmpfs | 15:04 |
| sinzui | CI doesn't use that feature, the only this job does | 15:04 |
| sinzui | I will ask if I can increase the artificial constraint | 15:05 |
| anastasiamac | sinzui: it'd b amazing! | 15:05 |
| bodie_ | sinzui, nice catch | 15:26 |
| sinzui | anastasiamac, I am trying your PR again | 15:27 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| sinzui | bodie_, resubmit. anastasiamac passed after the size incread | 15:45 |
| sinzui | increase | 15:45 |
| === rogpeppe2 is now known as rogpeppe | ||
| ericsnow | hazmat: you have any thoughts on what a plugin provider should look like? | 15:48 |
| ericsnow | hazmat: I have some and am writing up a spec | 15:48 |
| ericsnow | hazmat: I recall that you had done something like this already and wanted to get your thoughts | 15:48 |
| bodie_ | thanks sinzui | 15:49 |
| voidspace | ericsnow: providers don't have state do they, so my thinking was that we just take the existing providers and make them into an executable - with the existing api becoming commands | 16:03 |
| voidspace | ericsnow: marshall data in and out | 16:03 |
| ericsnow | voidspace: right | 16:04 |
| voidspace | ericsnow: you could prototype it over the weekend... ;-) | 16:04 |
| ericsnow | voidspace: haha | 16:04 |
| free | hello, I'm trying to play a bit with juju actions (trunk), but can't fully figure out how to get something working, anyone available to help? basically I always get "failed" as action status, but nothing I can see in the logs | 16:23 |
| voidspace | rogpeppe: ping | 16:30 |
| rogpeppe | voidspace: pong | 16:32 |
| voidspace | rogpeppe: I've updated a dependency - specifically gomaasapi | 16:33 |
| voidspace | rogpeppe: and I want to use the new version | 16:33 |
| voidspace | rogpeppe: dependencies.tsv for a bzr branch has revision number and some combination of email/date/hash | 16:33 |
| voidspace | where does the weird hash thing come from, seemingly not from bzr | 16:33 |
| voidspace | CONTRIBUTING.md suggests to run godeps -t $(go list github.com/juju/juju/...) > dependencies.tsv | 16:34 |
| voidspace | which changes a lot of things... | 16:34 |
| voidspace | but not the one I want | 16:34 |
| rogpeppe | voidspace: one mo, i'm in a call | 16:36 |
| voidspace | rogpeppe: no problem | 16:36 |
| voidspace | rogpeppe: I think I've solved it - or at least got godeps to generate the line and I'll revert the other changes | 16:36 |
| alexisb | ericsnow, well done, thank you very very much! | 16:37 |
| alexisb | I will send out a summary before my eod to you and perrito666 | 16:37 |
| ericsnow | alexisb: is there a mailing-list for the team leads? | 16:37 |
| ericsnow | alexisb: glad to do it | 16:37 |
| ericsnow | :) | 16:38 |
| alexisb | ericsnow, no, just my local one :) | 16:38 |
| ericsnow | :) | 16:38 |
| perrito666 | alexisb, ericsnow tx | 16:38 |
| alexisb | I will send you the address | 16:38 |
| ericsnow | alexisb: k | 16:38 |
| ericsnow | alexisb: I expect the lib will be fine (looks to be officially supported and uses the apache v2 license) | 16:39 |
| rogpeppe | voidspace: the approach i usually use is: godeps -u dependencies.tsv | 16:39 |
| rogpeppe | voidspace: then update the dependency you want | 16:39 |
| rogpeppe | voidspace: then godeps -t ./... > dependencies.tsv | 16:40 |
| rogpeppe | voidspace: except that currently doesn't work (annoyingly) in juju because of platform-specific deps | 16:40 |
| ericsnow | GOOS=windows godeps -t ./... > dependencies.tsv | 16:40 |
| ericsnow | and make sure your godeps is up to date :) | 16:40 |
| rogpeppe | ericsnow: does that get linux-only deps? | 16:40 |
| rogpeppe | ericsnow: (perhaps there aren't any...) | 16:40 |
| ericsnow | rogpeppe: I don't know | 16:40 |
| ericsnow | rogpeppe: I believe we only have windows-only deps | 16:41 |
| rogpeppe | i really need to sort out that godeps issue, but i haven't any allocated time for it... | 16:41 |
| ericsnow | nate would know better | 16:41 |
| voidspace | rogpeppe: right, thanks | 16:41 |
| rogpeppe | voidspace: i quite often do the cut&paste thing too, just to update a single line of dependency | 16:42 |
| ericsnow | rogpeppe: with newer godeps does that work right (due to the timestamp)? | 16:42 |
| rogpeppe | ericsnow: it should be fine | 16:42 |
| ericsnow | rogpeppe: good to know | 16:43 |
| rogpeppe | ericsnow: the time stamp is a deterministic function of the dependency | 16:43 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| free | does somebody know how action parameters are supposed to be accessed by the executable performing the action? some via juju env command? | 17:10 |
| ericsnow | wwitzel3: ready? | 17:13 |
| wwitzel3 | ericsnow: yep | 17:24 |
| ericsnow | wwitzel3: I'm in moonstone | 17:26 |
| cherylj | katco: ping | 17:29 |
| katco | cherylj: howdy! | 17:29 |
| cherylj | katco: do you have a few minutes? | 17:29 |
| katco | cherylj: sure, let me just grab a drink rq | 17:29 |
| voidspace | dimitern: hah, we had a maas provider test depending on the bug in the TestServer that I fixed | 17:31 |
| voidspace | dimitern: testing the fallback when it can't query architectures | 17:31 |
| dimitern | voidspace, really? :) | 17:31 |
| voidspace | dimitern: it relied on getNodegroups erroring rather than returning an empty list | 17:31 |
| dimitern | voidspace, good catch! | 17:32 |
| voidspace | dimitern: so I've changed the production code to treat an empty list of boot images to be the same as an error | 17:32 |
| voidspace | and use the fallback for supported architectures | 17:32 |
| voidspace | dimitern: I don't think it matters in practise | 17:32 |
| dimitern | voidspace, I'd have a look when you propose it | 17:33 |
| voidspace | dimitern: sure | 17:33 |
| voidspace | dimitern: it's easy to see what I mean | 17:33 |
| voidspace | dimitern: nearly ready for proposal | 17:33 |
| katco | cherylj: ok sorry, what's up? | 17:33 |
| dimitern | voidspace, cheers | 17:33 |
| cherylj | katco, I had some questions for you. Want to do a hangout? | 17:33 |
| katco | cherylj: sure | 17:34 |
| === kadams54 is now known as kadams54-away | ||
| === urulama is now known as urulama_ | ||
| katco | hey can someone add cherylj to the juju team on github? | 18:18 |
| katco | alexisb: ^ | 18:19 |
| alexisb | katco, let me see if I have admin to do that one sec | 18:19 |
| katco | alexisb: i think you are in the owners group, so i think you can :) | 18:19 |
| alexisb | cherylj, what is you gitbub user name? | 18:24 |
| cherylj | cherylj | 18:24 |
| katco | alexisb: cherylj | 18:24 |
| alexisb | cherylj, katco sent an invite let me know if that is what you need | 18:26 |
| cherylj | alexisb, got it! Seems to have worked | 18:28 |
| alexisb | coolness :) | 18:29 |
| alexisb | thanks for forcing me to learn something new | 18:29 |
| alexisb | :) | 18:29 |
| cherylj | thanks again, katco for helping me make my first (trivial) change! | 18:38 |
| katco | cherylj: congrats :D | 18:38 |
| katco | cherylj: you're now an official juju hacker :) | 18:39 |
| cherylj | yay! :D | 18:39 |
| katco | cherylj: oh i forgot to mention: the review board ticket will now be closed as well. ericsnow's integration does that for us as well. | 18:39 |
| katco | cherylj: you can always get back to it by looking at your "Outgoing > All" header there | 18:39 |
| ericsnow | cherylj: \o/ | 18:39 |
| cherylj | katco: cool, thanks! | 18:39 |
| katco | cherylj: well, it won't be closed until the juju bot closes your PR | 18:40 |
| katco | ok, time for nom nom nom... bbiab | 18:40 |
| voidspace | right | 18:41 |
| voidspace | happy weekend everyone | 18:41 |
| voidspace | EOW | 18:42 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === urulama_ is now known as urulama | ||
| lazyPower | What kicks off the mongodb process for the local provider? is this an upstart job that i'm failing to find or is this controlled by the juju bootstrap sequence? | 19:58 |
| katco | sinzui: hey this looks very strange to me: http://juju-ci.vapour.ws:8080/job/github-merge-juju/1767/consoleFull | 19:59 |
| katco | sinzui: is something the matter with the environment maybe? | 19:59 |
| katco | lazyPower: i think it's the bootstrap process? i don't think it's configured as a service | 20:00 |
| lazyPower | katco: 10-4, we're running down the issues with kicking off a juju env in a docker container - and this is where we're stuck | 20:01 |
| lazyPower | mongod isn't starting - so now we're looking for the mongo logs to figure out why thats the case - the $JUJU_HOME/local isn't being created | 20:01 |
| lazyPower | thanks for the feedback tho - *hattip* | 20:01 |
| mgz | katco: that one looks like the normal mongo falling over with bad record MAC then everything else being seriously ill afterwards | 20:02 |
| katco | lazyPower: ah i think i ran into this like 7mo ago, and docker containers didn't have the infrastructure to start... services. so mayb it is a service? | 20:02 |
| katco | mgz: so flakey test? i hadn't seen this one yet (i don't think...) | 20:02 |
| mgz | lazyPower: running local env inside docker seems... perverse | 20:02 |
| mgz | katco: yeah, tell her to retry it | 20:03 |
| katco | lazyPower: i just remember it having to do with how docker containers interacted with linux's init system (upstart or possibly something in /dev) | 20:03 |
| lazyPower | mgz: but it has its merits in terms of handing someone a docker container to get them looking at juju. | 20:03 |
| katco | mgz: will do, thank you (and happy new year btw!) | 20:03 |
| mgz | :) | 20:03 |
| lazyPower | mgz: when we get this figured out, we'd love your feedback on the project | 20:04 |
| lazyPower | +1 or -1 alike - we're in this for the community | 20:04 |
| mgz | lazyPower: and ideally our local setup would be less of a special unicorn and not have a magic machine 0, but that's where we're at currently | 20:04 |
| lazyPower | mgz: i knowwwwww | 20:05 |
| lazyPower | mgz: we want a container for bootstrap so bad here in ecosystems, but well be patient | 20:05 |
| lazyPower | *we'll | 20:05 |
| lazyPower | we know there are bigger fish to fry | 20:05 |
| katco | lazyPower: fwiw i think core wants that pretty badly too | 20:06 |
| katco | but as you said it's not high on the list | 20:06 |
| lazyPower | yeah, I try not to fishbowl too much | 20:09 |
| lazyPower | but i'm guilty of it | 20:09 |
| whit | katco, lazyPower : $JUJU_HOME/local is created. it just gets cleaned up when the bootstrap process fails | 20:27 |
| lazyPower | ahhh | 20:27 |
| lazyPower | good 2 know, is there a way to freeze it from getting reaped? | 20:27 |
| lazyPower | s/reaped/cleaned up/ | 20:27 |
| whit | what we need is a list of dependencies that jujud, mongod and friends expect for local bootstrap | 20:28 |
| whit | lazyPower, --keep-broken | 20:28 |
| lazyPower | niiiiice | 20:28 |
| katco | cherylj: resubmit again | 20:29 |
| cherylj | katco: okay :) | 20:29 |
| * perrito666 changes locations to get freshness | 20:30 | |
| perrito666 | ok, apparently 37ºC is not something that can be dodged by going to the 0th floor | 20:45 |
| cherylj | katco: Success! Huzzah! | 20:49 |
| katco | cherylj: lol yaaay | 20:50 |
| cherylj | 3rd time's a charm | 20:50 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1409141 | ||
| sinzui | anastasiamac, I believe a test introduced in one of your revisions is bad, not your code, but your test. I reported bug 1409141 about ppc test failure | 22:01 |
| mup | Bug #1409141: TestSetEntitiesAnnotation fails on ppc64el <ci> <regression> <test-failure> <juju-core:Triaged> <https://launchpad.net/bugs/1409141> | 22:01 |
| anastasiamac | sinzui: m looking. thnx! | 22:13 |
| anastasiamac | sinzui: :( | 22:13 |
| anastasiamac | sinzui: btw, thnx for landing my branch! | 22:14 |
| sinzui | anastasiamac, I just sent an email about the bug to the list. Since CI has a queue to test, I am going to wait until tomorrow to forking 1.22. | 22:14 |
| anastasiamac | sinzui: sorry :( | 22:15 |
| sinzui | anastasiamac, I hope this gives you time to adjust the test | 22:15 |
| perrito666 | anastasiamac: \o/ congratulations your first CI breakage, it took too long | 22:15 |
| sinzui | anastasiamac, you didn't create the queue@ | 22:15 |
| anastasiamac | sinzui: i'll look at it a bit later than - kids r going nuts :( must be saturday | 22:15 |
| perrito666 | anastasiamac: is it already saturday for you? | 22:16 |
| anastasiamac | perrito666: :( yes | 22:16 |
| perrito666 | anastasiamac: wow | 22:16 |
| anastasiamac | perrito666: i grow older faster here | 22:16 |
| perrito666 | anastasiamac: lol | 22:16 |
| anastasiamac | sinzui: i believe the problem is ordering - I say that I expect thing in one collection but the ordeer returned is reversed. | 22:19 |
| anastasiamac | sinzui: i don't care about the order so will try to fix now ;) | 22:19 |
| === urulama is now known as urulama__ | ||
| anastasiamac | sinzui: proposed fix is on reviewboard http://reviews.vapour.ws/r/706/ | 22:38 |
| * katco looking | 22:39 | |
| anastasiamac | katco: thnx :) | 22:47 |
| katco | anastasiamac: a few comments as i was trying to understand the code, and 1 spelling correction | 22:49 |
| katco | anastasiamac: other than that, i'll lgtm | 22:49 |
| anastasiamac | katco: about setting collection inline in other collections instead of declaring them as separate variables first | 22:51 |
| anastasiamac | katco: i find it hard to read and maintain if struct changes | 22:51 |
| katco | anastasiamac: it's just a map within a map right? | 22:52 |
| anastasiamac | katco: which was the case for me until the format of SetAnnotations was decide | 22:52 |
| anastasiamac | katco: yes it is now but went thru couple of changes | 22:53 |
| katco | anastasiamac: ah ok | 22:53 |
| katco | anastasiamac: well, whatever works for you :) it's certainly fine as is | 22:53 |
| katco | anastasiamac: the function declaration is actually confusing for me, because there's no change in indentation from the declaration and its body | 22:53 |
| katco | anastasiamac: nor whitespace | 22:54 |
| anastasiamac | katco: I'll add whitespace | 22:57 |
| anastasiamac | katco: had to params on separate linesotherwise signature is too long | 22:58 |
| katco | anastasiamac: consider breaking after last param. indentation looks like a struct and is more readable | 22:58 |
| anastasiamac | katco: thnx ;) that's what I meant | 22:58 |
| katco | anastasiamac: also, i have not seen anyone leave the first line of the func declaration blank like that "func(\n\n..." | 22:58 |
| anastasiamac | katco: oh k - i would have thought it's logical to move each param on its own line :P | 23:00 |
| anastasiamac | katco: i'll move one to func( | 23:00 |
| katco | anastasiamac: ty! | 23:00 |
| anastasiamac | katco: thnx for looking! updated review :) | 23:01 |
| anastasiamac | katco: can u lgtm? | 23:02 |
| katco | anastasiamac: did you forget the newline before ) error {, or was that intentional? | 23:02 |
| katco | L37 | 23:02 |
| anastasiamac | katco: actually, my IDE tells me that it's not right to have return on a separate line. so intentional | 23:04 |
| katco | anastasiamac: you just need an additional comma after the last param | 23:04 |
| wwitzel3 | ericsnow: almost back :) | 23:05 |
| ericsnow | wwitzel3: k | 23:05 |
| === kadams54 is now known as kadams54-away | ||
| anastasiamac | katco: i c. in that case, why would I want a newline after declaration? The indentation looks k now ;) let me commit for ur perusal | 23:06 |
| anastasiamac | katco: committed | 23:06 |
| katco | anastasiamac: yeah with the indentation for the return type, no need for newline after declaration | 23:06 |
| ericsnow | wwitzel3: I'm in moonstone when you're ready | 23:07 |
| katco | anastasiamac: that looks much better... did that run through gofmt? the indentation looks strange in the declaration as well | 23:07 |
| katco | anastasiamac: like the parameters are further in than the 1st param on my screen | 23:07 |
| === kadams54-away is now known as kadams54 | ||
| anastasiamac | katco: i believe it did. i have to go in a few min. can i get a lgtm? | 23:10 |
| katco | anastasiamac: yup, thanks for humoring me | 23:10 |
| anastasiamac | katco: anytime :) | 23:10 |
| katco | anastasiamac: fwiw, play.golang.org also keeps that indentation style. that seems weird to me... | 23:11 |
| anastasiamac | katco: m relying on my IDE to guide me as far as formatting (since it's all hooked in) | 23:11 |
| perrito666 | you know, if we change lgtms for amen we could get funny sentences more often | 23:11 |
| katco | haha | 23:11 |
| katco | perrito666: check out https://github.com/anastasiamac/juju/blob/set-annotations-Bug1409141/api/annotations/client_test.go | 23:12 |
| anastasiamac | katco: m not usually too fussed about formatting (coming from Java and all) but by all means pull me up on it when needed | 23:12 |
| katco | doesn't L34-37 looks weird indentation wise? | 23:12 |
| katco | anastasiamac: i usually try to ignore formatting in reviews, but that one just got me. every time i tried to scan for the function i lost track of where it was | 23:13 |
| perrito666 | it does yet I know why that happened | 23:13 |
| katco | perrito666: please enlighten me, amen. | 23:13 |
| perrito666 | go fmt sucks at fixing multiline function declarations | 23:13 |
| katco | haha | 23:13 |
| perrito666 | it will do the right thing with the bracket | 23:14 |
| anastasiamac | perrito666: :) | 23:14 |
| perrito666 | which will screw the rest | 23:14 |
| katco | well that is rather frustrating | 23:14 |
| perrito666 | or the parens I cant recall | 23:14 |
| perrito666 | its like one of the rules breaks the rest | 23:14 |
| perrito666 | I usually make declarations one line | 23:14 |
| perrito666 | to avoid these things | 23:15 |
| wwitzel3 | ericsnow: sorry for the delay, it was late enough I ended up having to make dinner instead of just lunch for myself :/ | 23:15 |
| ericsnow | wwitzel3: no worries :) | 23:15 |
| katco | perrito666: anastasiamac: http://play.golang.org/p/jKrQB2HQbg | 23:16 |
| katco | looks a little better | 23:16 |
| katco | if you drop the func onto a line by itself, that's when the formatting goes wacky: http://play.golang.org/p/INbYmRMf3- | 23:17 |
| anastasiamac | katco: yes when each param is on its own line | 23:17 |
| katco | anastasiamac: the difference being the "func(" isn't on it's own line | 23:17 |
| perrito666 | actually its when there is ( and ) in different lines | 23:18 |
| perrito666 | indentation of ) will make all look weird | 23:18 |
| anastasiamac | perrito666: katco: thnx for ur help! something new every day :P | 23:25 |
| katco | anastasiamac: thank you for taking the time on your sat. to fix that! | 23:25 |
| anastasiamac | katco: i broke it :) | 23:25 |
| anastasiamac | katco: besides it ain't fixed until it's merged | 23:26 |
| katco | anastasiamac: isn't that the hard truth :p | 23:26 |
| perrito666 | anastasiamac: don't punish yourself, breaking the build happens, a lot, to everyone | 23:26 |
| anastasiamac | katco: thnx for being my ocr on this ;) | 23:27 |
| katco | anastasiamac: i am biased towards tanzanite members ;) | 23:27 |
| anastasiamac | perrito666: m not punishing myself :) just taking ownership :) | 23:27 |
| anastasiamac | katco: i knowthe feeling! | 23:27 |
| * perrito666 notices the matrix 2 is playing in the background... man those are cheap cgis | 23:28 | |
| perrito666 | I wonder how much time has the TV been on | 23:29 |
| anastasiamac | merge failed: "upgrade in progress - Juju functionality is limited" | 23:31 |
| * anastasiamac will re-try | 23:31 | |
| perrito666 | anastasiamac: yup, that is a race | 23:31 |
| anastasiamac | perrito666: who is winning? | 23:31 |
| anastasiamac | perrito666: did i create t or shall I just re-try? | 23:32 |
| perrito666 | anastasiamac: nop, always been there | 23:32 |
| perrito666 | just retry | 23:32 |
| anastasiamac | perrito666: thnx :0 will do | 23:32 |
| anastasiamac | fix merged. hopefully this will unblock trunk eventually :) | 23:51 |
| perrito666 | anastasiamac: we will fix it if it doesn't please go enjoy your weekend | 23:52 |
| perrito666 | btw, what time on saturday is? | 23:52 |
| anastasiamac | perrito666: now almost 10am :) | 23:53 |
| anastasiamac | perrito666: what time is it for u? | 23:53 |
| perrito666 | did you pull an allnighter? | 23:53 |
| perrito666 | 20:53 on friday | 23:53 |
| anastasiamac | perrito666: no ;-) my son is helping with Asian Cup (soccer) and ws too existed to sleep so we've been up since 5am | 23:53 |
| anastasiamac | perrito666: and m more of an owl than an early bird - i work beta in the evening so I usually stay up til at leats 1am :) | 23:54 |
| anastasiamac | perrito666: at least* | 23:54 |
| anastasiamac | however, now that my fix is committed, I'll eow | 23:55 |
| perrito666 | well, enjoy | 23:58 |
| anastasiamac | perrito666: thnx ;) u2 | 23:58 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!