| davecheney | ok, one way to think about it is the defer runs _outside_ the scope of the function | 00:00 |
|---|---|---|
| davecheney | think of it like this | 00:00 |
| thumper | so the values returned in return statements are assigned prior to the defer running? | 00:00 |
| davecheney | outer() error { | 00:00 |
| davecheney | err := inner() { | 00:00 |
| davecheney | // the defer is here | 00:00 |
| davecheney | } | 00:00 |
| davecheney | yes | 00:00 |
| thumper | ok | 00:01 |
| davecheney | it's fucking confusing | 00:01 |
| thumper | that it is | 00:01 |
| davecheney | basically that sets up a defer at that point in the fuction that if anyone does return err after that | 00:01 |
| davecheney | st will be closed | 00:01 |
| davecheney | it could be done with an explcit, if err != nil { st.Close(); return err } | 00:02 |
| davecheney | at those three points after the defer is added | 00:02 |
| davecheney | thumper: what is even more confusing, you can do this | 00:03 |
| davecheney | func f() (err error) { | 00:03 |
| davecheney | defer func() { | 00:04 |
| davecheney | if err != nil { ... } | 00:04 |
| davecheney | } | 00:04 |
| davecheney | if err := something(); err != nil { | 00:04 |
| davecheney | return err | 00:04 |
| davecheney | } | 00:04 |
| davecheney | normally you'd thin that the err declared inside the if block shadows the one inside the defer block | 00:04 |
| davecheney | but in this spefic case it does not | 00:04 |
| thumper | because it is returned right? | 00:04 |
| davecheney | because err inside the defer block captures the value of err on the way out, no matter how it is return (either implicitly or explicitly) | 00:05 |
| * thumper nods | 00:05 | |
| davecheney | under the hood the defer is capturing the address of the value returned on the stack | 00:05 |
| davecheney | which it actaully easier to understand how it works | 00:05 |
| davecheney | err has an address on the stack | 00:05 |
| davecheney | it is assigned somehow by err = XXX, or return err | 00:06 |
| davecheney | then the defer can retrieve the value from the stack because it knows its name | 00:06 |
| * thumper nods | 00:06 | |
| davecheney | i don't think this is a very friendly pattern | 00:06 |
| davecheney | it's shorter, to write, but not to read | 00:06 |
| thumper | I think it is useful so you can't forget, or more importantly, that someone else comes back later, changes something and doesn't do the expected action | 00:09 |
| thumper | I don't like how it requires naming the return values | 00:09 |
| thumper | would be nice if there was another way to address them | 00:09 |
| thumper | menn0, davecheney: http://reviews.vapour.ws/r/39/diff/# | 00:20 |
| davecheney | This diff has been split across 2 pages: 1 2 > | 00:28 |
| davecheney | ^ warning | 00:28 |
| thumper | davecheney: geez that is annoying | 00:29 |
| thumper | davecheney: is that a setting in review board itself? | 00:29 |
| wallyworld_ | bradm: maybe tomorrow hopefully | 00:31 |
| davecheney | thumper: none that I have found | 00:33 |
| thumper | davecheney: perhays in the deployment of reviewboard, rather than a client setting... maybe | 00:34 |
| waigani | anyone know how to find a mongo doc missing a particular field? | 01:09 |
| waigani | I'm playing with bson and reading what docs I can find, no luck yet... | 01:09 |
| thumper | wallyworld_: I think you can fine based on unset | 01:28 |
| thumper | s/fine/find | 01:28 |
| wallyworld_ | wot | 01:28 |
| thumper | waigani: sorry, that was for you | 01:28 |
| thumper | wallyworld_: EWRONGNICK | 01:28 |
| waigani | thumper: thanks | 01:29 |
| waigani | bson.DocElem{"$unset", fieldName}) | 01:32 |
| axw | $unset is for removing a field | 01:49 |
| axw | you want a combination of $not and $exists, I think. | 01:50 |
| ericsnow | axw: that looks like Sunset, Snot, and Sexists | 01:57 |
| axw | :p | 01:57 |
| === fuzzy_ is now known as Ponyo | ||
| menn0 | thumper, davecheney: the documentation isn't very clear but it seems like RB's pagination can be tweaked: https://www.reviewboard.org/docs/manual/1.7/admin/configuration/diffviewer-settings/ | 02:47 |
| * rick_h_ watches thumper's kiwi pycon talk bwuhahaha | 03:03 | |
| thumper | rick_h_: hey... | 03:03 |
| thumper | rick_h_: go on, tell me what you really think | 03:04 |
| rick_h_ | 10min in so far | 03:04 |
| rick_h_ | 18 to go | 03:04 |
| rick_h_ | I'll write a critique later, especially with this django crap :P | 03:04 |
| bradm | oh, haha | 03:35 |
| bradm | that problem I had yesterday with 1.20.7, its the same with 1.18.1 | 03:35 |
| bradm | still does the ifdown eth0, which takes it off the network | 03:35 |
| menn0 | very easy review required: http://reviews.vapour.ws/r/61/diff/ | 03:42 |
| menn0 | thumper, axw, davecheney, wallyworld_?: ^^^ (it's a one line change) | 04:31 |
| thumper | menn0: done already | 04:31 |
| thumper | geez | 04:31 |
| menn0 | thumper: I just refreshed before. must have just missed you. sorry. | 04:31 |
| wallyworld_ | done | 04:31 |
| thumper | heh | 04:31 |
| * thumper needs to go get sushi before the world implodes here | 04:31 | |
| menn0 | thumper: why is the world imploding? | 04:33 |
| thumper | menn0: because jessie isn't getting her sushi | 05:37 |
| menn0 | thumper: right | 05:44 |
| menn0 | thumper: I thought it was bad weather coming in or something :) | 05:44 |
| === urulama-afk is now known as urulama | ||
| menn0 | davecheney and thumper: I think you might like this: http://reviews.vapour.ws/r/64/diff/ | 05:53 |
| thumper | is it deleting a lot of code? | 05:53 |
| thumper | page doesn't exist... | 05:54 |
| menn0 | thumper: no but it barely adds any | 05:54 |
| menn0 | thumper: try again ... I hadn't hit publish | 05:54 |
| menn0 | thumper: this avoids what will no doubt be a flaky CI test | 05:55 |
| menn0 | thumper: I started down that road but couldn't see a sane way to test this otherwise | 05:56 |
| thumper | ah, actually, I really do like that test :) | 05:56 |
| menn0 | sweet :) | 05:56 |
| menn0 | I was hoping davecheney might be able to look at the reflection bit. I'm wondering if there's a tidier way to get the method set for an interface. | 05:57 |
| * thumper needs alcohol | 05:58 | |
| thumper | menn0: I'm trusting that it does what it says on the can | 05:58 |
| thumper | menn0: I'll let davecheney critique the method of getting the method names :) | 05:59 |
| thumper | I'm about to go make a pizza, open a bottle of wine, and do some evening javascript hacking | 05:59 |
| thumper | night all | 05:59 |
| * menn0 is off to pick up pizza himself | 06:00 | |
| davecheney | menn0: i have no idea what that is supposed to do ? | 06:23 |
| TheMue | dimitern: ping | 08:15 |
| dimitern | TheMue, hey | 08:16 |
| TheMue | dimitern: as long as we use RB *smile* would you mind taking a look at http://reviews.vapour.ws/r/43/ | 08:17 |
| TheMue | dimitern: sadly the according PR doesn't know the changes due to the tools | 08:19 |
| TheMue | interesting, didn't expected this | 08:19 |
| dimitern | TheMue, that's probably because you didn't sync your parent master branch with upstream? | 08:20 |
| dimitern | TheMue, looking | 08:20 |
| TheMue | dimitern: that's what I would expect by a tool, not to do it manually (but I thought I had *hmmm*) | 08:21 |
| TheMue | dimitern: regarding my change I would like to diskuss something with you. it's about the usage of the baseSuite. I'm passing it as argument, to make the decoupling more explicit but surely all testAbcV0() to testXyzV999() could be written as methods of the baseSuite too. | 08:26 |
| dimitern | TheMue, sorry, expand a bit more ? | 08:40 |
| eagles0513875 | hey all :) | 08:41 |
| eagles0513875 | dimitern: just replied to the RB thread btw | 08:41 |
| eagles0513875 | with a suggestion :) | 08:41 |
| dimitern | eagles0513875, hey :) nice, thanks | 08:42 |
| eagles0513875 | no problem | 08:42 |
| eagles0513875 | i proposed custom coding something that will interface with githooks and be a web interface :) | 08:42 |
| eagles0513875 | and that if the team is interested is something I would like to work on | 08:42 |
| eagles0513875 | seeing as I am not versed in golang | 08:43 |
| eagles0513875 | dimitern: you still in malta? | 08:43 |
| eagles0513875 | morning fwereade :D | 08:43 |
| fwereade | eagles0513875, o/ | 08:44 |
| eagles0513875 | fwereade: i have a RB replacement suggestion not sure if you saw the reply i sent to the RB thread | 08:44 |
| fwereade | eagles0513875, I don't think that's really a good use of anyone's time -- there's no shortage of review systems in existence | 08:47 |
| fwereade | eagles0513875, http://sheep.art.pl/Unix%20Koan ;) | 08:47 |
| eagles0513875 | fwereade: agreed but with what you reviewed before setting on RB | 08:47 |
| eagles0513875 | what were the others lacking | 08:48 |
| eagles0513875 | the way i see it something custom coded I would be willing to create for you guys something that will integrate with the github hooks | 08:48 |
| eagles0513875 | i already have some good ideas on what can be done fwereade just have to see how feasible it would be to do them with the api and hooks | 08:49 |
| eagles0513875 | such as one files a pull request on github and then on said review platform they woudl submit the pull request if approved it would interface with github in a way you can push said reviewed PR through said review site | 08:50 |
| eagles0513875 | fwereade: one idea to start off with | 08:51 |
| eagles0513875 | either that or find a way where it would pick up on pending pull requests automatically | 08:51 |
| dimitern | TheMue, reviewed | 08:51 |
| dimitern | eagles0513875, no, I moved back home for a while | 08:51 |
| eagles0513875 | kool :) you arent missing much here in malta except its super hot still and humid and fwereade can attest to that | 08:52 |
| dimitern | hot and sunny.. nice it almost drowned us here lately with the rain | 08:52 |
| TheMue | dimitern: thanks | 08:53 |
| TheMue | dimitern: I would like to change the funcs to methods too, but all V0 are in the V0 file while later all new V2+ tests are in those files. or would you always place the V0 to V999 test methods into one file? that way surely a testFooV7() could be placed after testFooV1() and the change would be more abvious. | 08:57 |
| fwereade | eagles0513875, well, it's the chaining that seems to be the biggest issue across the board -- but regardless I worry you're seriously underestimating what it'd take to write a whole new review system | 08:59 |
| dimitern | TheMue, let's not do that :) too many things in one file | 08:59 |
| TheMue | dimitern: exactly | 08:59 |
| eagles0513875 | fwereade: out of many that are out there how many interface with the github api's | 08:59 |
| TheMue | dimitern: so V0 in V0 and Vx in Vx | 08:59 |
| TheMue | dimitern: but still as methods of the baseSuite | 09:00 |
| TheMue | dimitern: +1 | 09:00 |
| dimitern | TheMue, IMO it's easier that way - you immediately see what is where | 09:00 |
| TheMue | dimitern: yes | 09:00 |
| eagles0513875 | fwereade: i know its goign to be a monster of a task, buti think a project like that would benefit the github user community as a whole | 09:00 |
| TheMue | dimitern: I think we've got a good approach then | 09:00 |
| eagles0513875 | make it easier to review pull requests | 09:00 |
| eagles0513875 | etc | 09:00 |
| dimitern | eagles0513875, it doesn't have to be a whole system btw - if there's somehow a way to automate a way of chaining dependent PRs with the github api | 09:03 |
| eagles0513875 | dimitern: noted. this was an idea that just came to me :) | 09:03 |
| eagles0513875 | fwereade: see above ^ | 09:03 |
| eagles0513875 | dimitern: if you are going to create something for that might as well create something which has a working gui and its simple to use | 09:04 |
| * TheMue simply would like to stay with GH w/o any additional tool. less is more. | 09:04 | |
| eagles0513875 | TheMue: i know it is but if you need a review system that can help review the code a bit quicker and it interfaces directly with github why not | 09:05 |
| eagles0513875 | that way all you would need to do is file a pull request on GH then if possible the review system would poll pending pull requests and email devs saying this PR is pending review | 09:06 |
| eagles0513875 | once reviewed they coudl easily push it to the appropriate branch | 09:06 |
| TheMue | eagles0513875: an additional site, additional user management, additional usage, additional source of errors | 09:07 |
| eagles0513875 | agreed but what if userwise it does authentication against github accounts | 09:07 |
| TheMue | eagles0513875: but that's only my opinion, as a fan of KISS | 09:08 |
| eagles0513875 | TheMue: i am a fan of KISS too | 09:08 |
| eagles0513875 | that is why im thinking this review system would be as automated as possible with email notifications and interface with the github api to make things a bit easier | 09:08 |
| eagles0513875 | fwereade: any further thoughts or dimitern :) | 09:10 |
| fwereade | eagles0513875, I'm not really seeing anything other than a great big bunch of work that distracts from our core purpose | 09:11 |
| eagles0513875 | fwereade: not saying for you guys to do it | 09:12 |
| fwereade | eagles0513875, integrating *any* extra system is a great big hassle, and writing it from scratch as well renders it pretty much unfeasible imo | 09:12 |
| eagles0513875 | im volunteering myself to do it | 09:12 |
| fwereade | eagles0513875, well, sure, you can write anything you like, and I will wish you luck with it, but it doesn't have much bearing on our current situation | 09:12 |
| eagles0513875 | seeing as RB is rather un popular for certain things that is why i suggested somethign custom coded | 09:13 |
| fwereade | eagles0513875, it is by definition vaporware and hence not something for us to be basing plans on | 09:13 |
| eagles0513875 | is RB something open sourced? maybe I can get the source and improve on it for you guys | 09:13 |
| fwereade | eagles0513875, sure, but it's incredibly rare for the build/buy tradeoff to actually come down in favour of build *except* when it's about your core competencies | 09:14 |
| eagles0513875 | ya lost me | 09:14 |
| fwereade | eagles0513875, ok, how long does it take you on average to write a nice dynamic web app that interfaces with another site, from scratch? | 09:15 |
| eagles0513875 | fwereade: would be my first time | 09:15 |
| eagles0513875 | have an ideal place already in mind which gives a course about working with the github api | 09:15 |
| fwereade | eagles0513875, do you see why it would be somewhat foolhardy for us to depend on that? | 09:16 |
| eagles0513875 | agreed | 09:16 |
| fwereade | eagles0513875, like I say, if you're convinced it has value, go ahead, and I wish you much luck with it | 09:16 |
| eagles0513875 | fwereade: ill be honest i feel that is the only way i can contribute to juju at this point in time | 09:16 |
| fwereade | eagles0513875, I've been trying to steer you towards charm development for some time, AIUI that actually fits fairly well with your skillset | 09:17 |
| eagles0513875 | :) | 09:18 |
| eagles0513875 | question about that fwereade | 09:18 |
| eagles0513875 | where are bugs filed against charms on LP or bugs dont get filed against the charms | 09:18 |
| fwereade | eagles0513875, https://bugs.launchpad.net/charms | 09:19 |
| dimitern | mgz, ping | 09:21 |
| dimitern | mgz, is there a particular reason the merge bot runs out of disk space occasionally while running tests? | 09:21 |
| mgz | dimitern: I've not tracked it down yet | 09:31 |
| mgz | can only assume the temp drive allocation at the start where we ask for 2 gigs isn't working properly sometimes | 09:32 |
| mgz | but I don't see why it's now an issue where it didn't seem to be before | 09:32 |
| dimitern | mgz, right | 09:34 |
| === fabrice is now known as fabrice|lunch | ||
| TheMue | dimitern: quick talk? | 10:45 |
| dimitern | TheMue, sure, omv | 10:46 |
| perrito666 | morning | 10:47 |
| wallyworld_ | hazmat: meeting? | 11:05 |
| perrito666 | Changed a lot of things on a couple of files, go fmt says its all right </successkid> | 11:38 |
| perrito666 | so little is needed to make me happy... | 11:38 |
| perrito666 | is there a way to see the patch is going to be uploaded to rb befor doing so? | 11:50 |
| wallyworld_ | axw: katco: still in health/status meeting, running late | 11:59 |
| axw | okey dokey | 11:59 |
| axw | katco: wanna wait a bit? | 12:00 |
| wallyworld_ | axw: katco: feel free to have a chat and then finish without me | 12:04 |
| katco | axw: sorry just got my wife/daughter out the door | 12:05 |
| === fabrice|lunch is now known as fabrice | ||
| dimitern | fwereade, wallyworld_, perrito666, TheMue, others? please take a look at https://github.com/juju/juju/pull/799 - the firewaller worker changed to use the new api v1 supporting port ranges | 13:36 |
| TheMue | dimitern: PR on GH, not on RB? | 13:44 |
| * TheMue loves abbreviations :) | 13:45 | |
| dimitern | TheMue, yeah, forgot but it anyone insists, will add one :) | 13:47 |
| TheMue | dimitern: I don't need it, do it on GH | 13:47 |
| fwereade | dimitern, at first glance, shouldn't the upgrade step come first of all? | 13:58 |
| fwereade | dimitern, ah, right, maybe not | 13:59 |
| fwereade | dimitern, forget I said anything :) | 14:00 |
| natefinch | I think for truly trivial code reviews (under 10 lines or something), that GH is fine (like sinzui's version change ones). Anything more complicated should be on Reviewboard | 14:00 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Open critical bugs: 1371605 | ||
| dimitern | fwereade, yep :) | 14:03 |
| === luca__ is now known as luca | ||
| sinzui | natefinch, jam: can you get someone to look into bug 1371605 hp deplpys are broken in master | 14:11 |
| mup | Bug #1371605: HP Bootstrap fails: no endpoints known for service type: product-streams <bootstrap> <ci> <hp-cloud> <regression> <streams> <juju-core:Triaged> <https://launchpad.net/bugs/1371605> | 14:11 |
| natefinch | sinzui: will do | 14:14 |
| TheMue | fwereade: thx for doc review, just changing it, also due to the new approach of API testing | 14:33 |
| wwitzel3 | fwereade: http://reviews.vapour.ws/r/58/ already has a review from ericsnow, but needs a meta-review. | 14:33 |
| wwitzel3 | dimitern: http://reviews.vapour.ws/r/58/ set.Tags stuff we talked about if you also want to take a look :) | 14:34 |
| dimitern | wwitzel3, great, will do, thanks! | 14:34 |
| sinzui | natefinch, do you have a moment for https://github.com/juju/juju/pull/800 | 14:42 |
| === allenap_ is now known as allenap | ||
| natefinch | lol... just noticed .wtf is a valid TLD now | 15:13 |
| fwereade | wwitzel3, ericsnow: review LGTM, I'm still interested to know the motivation | 15:28 |
| === Ursinha-afk is now known as Ursinha | ||
| natefinch | fwereade, wwitzel3, ericsnow: I agree... do we really need a full-on set implementation for tags? | 15:31 |
| === TheRealMue is now known as TheMue | ||
| === katco` is now known as katco | ||
| fwereade | natefinch, well, if we do want sets of tags, then I'm not against carrying a Set implementation -- but yeah, I'm very interested to know the use case ;) | 15:35 |
| wwitzel3 | fwereade, natefinch: well mainly it was for dealing with user input. when a user provides many units and services as targets, you end up with duplicates. I had helpers that were filtering the list of names.Tag based on their strings, it ended up being 70% of a set implemtation anyway | 15:48 |
| wwitzel3 | fwereade, natefinch: so I figured, instead of having these private helpers, jsut do a set implementation for names.Tag, I felt is better addressed the problem and resulted in a cleaner implementation in the runcmd API | 15:49 |
| fwereade | wwitzel3, heh, interesting | 15:49 |
| fwereade | wwitzel3, is that to happen server-side or client-side? | 15:49 |
| fwereade | actually surely it's client-side | 15:50 |
| fwereade | wwitzel3, in which case wouldn't set.Strings on the initial input work just as well? | 15:51 |
| wwitzel3 | fwereade: services expand to multiple units server side | 15:51 |
| fwereade | wwitzel3, ahhhhh | 15:51 |
| fwereade | wwitzel3, cool, thanks | 15:51 |
| wwitzel3 | fwereade: np | 15:51 |
| wwitzel3 | fwereade: it is really just a matter of where did the work of "is this tag.String unique for this list" .. and I think using a set abstraction just made all the logic in the runcmd API very simple and easy to understand. | 15:55 |
| fwereade | wwitzel3, yep, I'm satisfied :) | 15:55 |
| === fabrice is now known as fabrice|dinner | ||
| mattyw | folks, I'm of, have a great weekend all | 16:51 |
| === fuzzy_ is now known as Ponyo | ||
| perrito666 | ericsnow: having a different client for the api for backups is not that happy :p | 22:35 |
| ericsnow | perrito666: why not? | 22:36 |
| perrito666 | ericsnow: I now need functionality from the regular api client and I will have to port it :p | 22:36 |
| ericsnow | perrito666: what functionality do you need? | 22:37 |
| perrito666 | for now apparently only client.PublicAddress | 22:37 |
| perrito666 | well Ill address that on monday | 22:40 |
| ericsnow | is that only on api.Client? | 22:41 |
| ericsnow | k | 22:41 |
| perrito666 | ericsnow: It makes some noise in my head to duplicate that but as long as its only that Ill be fine with it | 22:45 |
| ericsnow | perrito666: yeah, it makes me think that functionality is in the wrong place | 22:45 |
| perrito666 | ericsnow: ill have to think about it, we could make it better but its not worth it as it would most likely have a new level of abstraction, anyway have to run, have a nice weekend | 22:47 |
| ericsnow | perrito666: you too | 22:47 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!