/srv/irclogs.ubuntu.com/2014/05/28/#juju-gui.txt

rogpeppemornin' all08:19
frankbanhi rogpeppe 08:25
rogpeppefrankban: hiya08:28
frankbanrogpeppe, fwereade: isolation suite stuff proposed in https://github.com/juju/testing/pull/509:01
rogpeppefrankban: looking09:02
frankbanthanks09:02
rogpeppefrankban: if you're going to have a new package (not a bad thing), perhaps isolation.Suite might be a better name09:03
rogpeppefrankban: although testing.IsolationSuite would probably work fine too09:03
frankbanrogpeppe: testing.IsolationSuite introduces an import loop09:04
rogpeppefrankban: what's the loop?09:04
frankbanrogpeppe: logging requires testing, isolation (testing) requires logging09:05
rogpeppefrankban: logging requires testing? how come?09:05
frankbanrogpeppe: the old LoggingSuite (that we need to preserve for backward compatibility) embeds testing.CleanupSuite09:06
rogpeppefrankban: that seems spurious to me09:07
rogpeppefrankban: can't we change all the places that use LoggingSuite to use IsolationSuite instead?09:07
frankbanrogpeppe: we can, but that would require fixing all the  LoggingSuite tests using env vars. I guess we want to eventually do it anyway, but the idea is to have an incremental step in the direction of our current goal: separate utils/*, charm etc from juju-core/testing09:11
rogpeppefrankban: alternatively, just pull LoggingSuite into testing. it's easy to break out later.09:12
rogpeppefrankban: and it's all pretty trivial stuff09:13
frankbanrogpeppe: I like this more, then we have everything into testing: testing.CleanupSuite, testing.Logging(Only)Suite and testing.IsolationSuite09:14
rogpeppefrankban: that sounds good to me09:14
frankbanrogpeppe: +1, I'll do that. Please let me know when your review is done, and I'll start moving thos eparts09:16
fwereadefrankban, rogpeppe: yeah, that sounds reasonable, it doesn't feel like such an awfully large package -- but I note that it's this sort of thing that causes packages to gradually grow (not wanting to create lots of tiny ones) and makes them hard to split up (because the loops often mean you can't split off *one* package, you have to split off two or three)09:28
fwereadefrankban, rogpeppe: but I reiterate that in this case I think it's ok09:28
rogpeppefwereade: i'd like to move towards loggingsuite not embedding cleanupsuite, and then we can factor out things.09:29
rogpeppefwereade: i agree that it's not great that testing is likely to accumulate a big bag of deps09:30
fwereaderogpeppe, yeah, that is the plan09:30
fwereaderogpeppe, but starting by preserving LoggingSuite is the chosen first step on that path09:31
=== luca__ is now known as luca
frankbanfwereade, rogpeppe: MP updated, and it also seem juju-core must be changed in only one place to use the new logging suite path09:39
rogpeppefrankban: i'm not that familiar with github comments - i'm *thinking* that each comment is published the moment I click "Comment on this line", but please let me know if you haven't seen any review comments.10:13
frankbanrogpeppe: yeah github comments are "real time" (and I don't like that since I usually iterate a lot when reviewing)10:15
rogpeppefrankban: yeah, i agree - i very often go back and change mments10:15
rogpeppeearlier comments10:16
frankbanrogpeppe: except for juju-core, are we aware of other projects using the LoggingSuite?10:16
rogpeppefrankban: no, i'm pretty sure that juju-core is the only one (well, some of my stuff might do, but ignore that)10:16
frankbanrogpeppe: just another question about your comments: I know the fixture methods are just methods, I used embedding because it seemed sane to me to rely on the gocheck runner, without simulating its behavior (which I agree is not likely to change, but who knows...)10:18
rogpeppefrankban: i don't think that calling the method is "simulating behaviour" to any meaningful degree. if gocheck doesn't call the methods, we're stuffed, so unit testing by calling the methods seems fine to me.10:20
frankbanrogpeppe: ok I am convinced, this way I can also separate the two tests there10:21
rick_h_morning all10:45
frankbanrogpeppe: re-proposed with the changes10:49
frankbanrick_h_: morning10:49
rogpeppefrankban: looking10:50
rogpepperick_h_: hiya10:50
frankbanthanks10:50
rogpeppefrankban: do you know if it's possible to see just the new changes, with the comments shown too?10:52
rogpeppefrankban: (like codereview allows)10:53
frankbanrogpeppe: I don't think it's possible, there is no history when rebasing. I agree reitveld is much better10:53
rick_h_the comments are still there and you can click on them to see the old code that was commented on10:54
rogpeppefrankban: i mean, it doesn't matter much here, but in more complex changes, it's really useful to be able to see the comments and the changes that were made in response.10:54
rick_h_but you don't get just the diff in the new changes10:54
rick_h_it's an argument for saving rebase until landing I suppose. 10:54
rogpepperick_h_: so it's not actually possible to see what changes have been made since a comment (without downloading the patch directly, i guess)10:54
rogpeppe?10:54
rick_h_rogpeppe: yea, so if we didn't rebase down until after you got the :+1: then you could view commit by commit10:55
rogpeppeFWIW, that was something that i would do on almost every review10:55
frankbanrogpeppe: that's unfortunate in github10:55
rick_h_it's something we can talk about. 10:55
rick_h_rogpeppe: yea, I've been trying to find time to investigate running reviewboard in our CI environment to help but time hasn't been overflowing lately. 10:56
frankbanrick_h_: that's an idea, assuming the final rebasing does not require reviewers eyes10:56
rick_h_frankban: yea, in theory it shouldn't10:56
rogpepperick_h_: could you view that on github itself? or would you need to pull down the (unrebased) patch?10:56
rick_h_rogpeppe: reitveld left a lot of sour tastes in people's mouth though much of that blame lies more in LP->reitveld10:56
rick_h_rogpeppe: so a partial step would be a policy not to rebase your branch down until just before you submit it for merging10:57
rogpepperick_h_: yeah, the lp->rietveld bridge was a bit awkward10:57
rick_h_rogpeppe: that would be in GH itself and would get you much closer to what you want10:57
rick_h_frankban: I'll add a friday card and you guys can discuss?10:57
frankbanrick_h_: sure, +1 thanks10:58
rogpepperick_h_: i'd find that useful, i think - for complex changes it can be very hard to see what changes have been made in response to a comment, and whether the correspond with what's wanted10:58
rogpeppes/the correspond/they correspond/10:58
rick_h_rogpeppe: understood, it's something we fight with LoC limits and multiple reviews required to land on larger branches10:59
rick_h_rogpeppe: so we don't always feel that pain ime10:59
rogpepperick_h_: what are the LoC limits, BTW?10:59
rick_h_in the GUI 400 LoC and less you need one review and one QA11:00
rick_h_over 400 LoC you need two reviews and one QA minimum...possibly 211:00
rick_h_over 800 ish LoC you need to break it down or bribe reviewers and beg for forgiveness11:00
rick_h_rogpeppe: that is more diff LoC than actual LoC 11:01
rick_h_LoD (lines of diff)11:01
rogpepperick_h_: that's including context, right?11:01
rick_h_rogpeppe: yea11:02
rogpepperick_h_: so a one line change would be ~10 LoD11:02
rick_h_it's usually closer to 5ish but yea11:02
rick_h_hmm, GH doesn't report the real diff size does it. Looking at https://github.com/juju/juju-gui/pull/348 it just has the +69/-1411:03
rogpepperick_h_: i get 9 lines, not including the file header11:03
rick_h_oh yea, hover it I see11:03
rick_h_so https://github.com/juju/juju-gui/pull/348 is an 83 LoC diff technically11:03
rick_h_rogpeppe: ok, sounds good then. Not normally checking the size of a one line change. It's under any limits :) Then you just need to figure out if self-review is possible. 11:04
rogpepperick_h_: the 83 lines looks like 69+14, not the diff output line count11:04
rogpepperick_h_: i suspect git diff | wc will be considerably larger11:04
rick_h_rogpeppe: ah right bah11:04
rick_h_rogpeppe: I guess it's just habit these day. We're used to breaking down small enough. You're right that we don't currently have a good number in GH like we did in LP11:05
* rick_h_ makes a mental note to figure out how to port the old rules to GH 11:06
rogpepperick_h_: tbh, i prefer lines without context - it doesn't penalise changes which change quite a few individual lines in a simple way11:06
frankbanFWIW my branch in review has +204  −67 and "git diff master | wc -l" returns 42511:07
frankbanrogpeppe: the magic calldepth value was pre-existing, I don't have that answer, if you do I'd be happy to add a comment there11:09
* rogpeppe hates the "publish immediately" thing11:10
frankbanrogpeppe: heh, ok I'll add a todo11:10
frankbanrogpeppe: also, when a review is completed, we usually add a final comment (not inline) so that the author knows when to start looking for comments11:11
rick_h_frankban: yea, we used to have different limits based on language back in launchpad. JS could have more than python in that case. 11:11
rick_h_+1 for the overall comment after all the in-lines are done11:12
rogpeppefrankban: where do you add that final comment?11:12
frankbanrick_h_: yes, also go diffs can be longer than python ones (gofmt stuff etc)11:13
rogpeppefrankban: oh, i see, on a different pae11:13
rogpeppepage11:13
frankbanrogpeppe: yeah, in the conversation tab11:13
rogpeppei think there should be an exception for low complexity diffs, particularly mechanically generated changes11:13
rick_h_definitely11:13
rogpeppefor instance, in Go, you may well change an import path which might affect 1000s of lines, but it can still be a trivial change11:14
rogpeppeit's got to be a judgement call in the end, i think11:14
frankbanyeah11:14
frankbanrogpeppe: how do you merge stuff in github? just use the big green button?11:16
rogpeppefrankban: i've no idea :-)11:16
rogpeppefrankban: i am an utter git newbie11:16
frankban:-) I'll ask on #juju-dev11:16
rick_h_yea, just hit the green button11:17
rick_h_until we get the libraries in CI and using the :shipit: lander11:17
frankbanrick_h_: cool, merged11:22
frankbanrogpeppe: ok, golxc uses logging.LoggingSuite, I'll update that and then juju-core11:31
* rick_h_ heads to take the boy to day care11:33
rick_h_rogpeppe: if you get a sec can you add a card to the board please for your current work?11:33
rogpepperick_h_: ah yes, will do11:34
frankbanrogpeppe: could you please take a quick look at https://code.launchpad.net/~frankban/golxc/fix-logging-suite/+merge/221210 ?11:51
rogpeppefrankban: not on codereview?11:51
frankbanrogpeppe: no lbox files in golxc11:52
rogpeppefrankban: you can still use lbox11:52
frankbanrogpeppe: you are right, let me create that11:53
rogpeppelbox propose -for lp:golxc/trunk11:53
frankbanrogpeppe: https://codereview.appspot.com/9266004611:54
rogpeppefrankban: i'm not sure about hardcoding /usr/bin there11:55
rogpeppefrankban: who's to say whether that's the location that the user is using for their lxc binaries?11:56
rogpeppefrankban: i think that probably commandArgs.SetUpSuite should save the value of $PATH, then TestCreateArgs etc can restore it11:57
frankbanthe errors are like this: http://pastebin.ubuntu.com/7536121/11:58
frankbanrogpeppe: +1 on saving the PATH12:01
frankbanrogpeppe: updated12:07
hazmatfrankban, afaics bug 1324097 is still an issue (just filed that)12:16
_mup_Bug #1324097: charm get api doesn't work with store charms <juju-core:New> <https://launchpad.net/bugs/1324097>12:16
rick_h_hazmat: cool will add that to our maintenance backlog to look at12:17
rick_h_hazmat: or is that related to the recent refactor work?12:17
hazmatrick_h_, its been a bug in that implementation since it was done12:18
rick_h_hazmat: ok cool, added to our on deck maint. 12:18
frankbanhazmat: we are now working on breaking the store into a separate project. rick_h_: is the goal to have the store itself expose an API for getting charm files?12:26
rick_h_frankban: yes, it'll be part of the api v412:27
hazmatfrankban, not related12:27
rick_h_frankban: speaking of going to share a doc with you to keep an eye on12:28
hazmatfrankban, unless your going to remove the broken api method12:28
rick_h_frankban: but as hazmat says, this is more a store bug issue and with us working on the store it's something we should keep an eye on and be willing to update as we bring up ownership. 12:28
rick_h_frankban: and things like this will be great for bringing up new go/store devs12:29
frankbanhazmat: that's what I was asking, we'll take a look at that, thanks12:29
frankbanrick_h_: +112:29
hazmatits not really a store api atm, the bug is against the state server api.. 12:34
hazmatrick_h_, apiv4 of charmworld? what's the delta on that? i was looking at implementing v3 api against the env to be able to support demos12:34
rick_h_hazmat: TBD12:34
hazmatic12:35
rick_h_hazmat: once we pull the store out we'll be working on defining api v4 to fix issues with v3, add current store functions required, and adding new ones for bundles/etc so that they're loaded like charms12:35
hazmatrick_h_, what version of osx were you on?12:48
hazmatre bug 132380312:48
_mup_Bug #1323803: ssl handshake fails on osx  <juju-deployer:New> <python-jujuclient:New> <https://launchpad.net/bugs/1323803>12:48
rick_h_hazmat: hmm, latest? /me checks12:49
rick_h_10.9.312:49
rick_h_hazmat: and the openssl version matched the linked url discussion12:49
hazmatrick_h_, so this is an osx and python issue.12:52
rick_h_hazmat: yes12:52
rick_h_hazmat: and webcocket library issue perhaps12:52
rick_h_hazmat: I chased it down enough to get close but filed a bug from there. Was trying to help that stein guy get notes on doing his demo stuff from osx and it's more work than I've got time for before I leave tomorrow12:53
hazmatrick_h_, no.. its deeper than that12:53
rick_h_hazmat: I've added it to our board to look into as we'll hit this with our current work to make juju-quickstart work on osx12:53
rick_h_hazmat: right, but it sounds like there might be config/ways to enforce ssl3 at handshake?12:53
rick_h_and skip ssl212:53
hazmatrick_h_, its an libssl issue.. the best fix we can do without modifying python or osx version of ssl12:53
hazmatis to change the server to only do ssl3 or tlsv112:54
rick_h_hazmat: cool, yea I'm not sure if there's a decent work around for the client or it requires server changes. I figure we can't be the first to hit this stuff (and a google search proves we're not)12:55
hazmatrick_h_, there is no workaround for the client12:55
hazmatwithout changing python versions, patching py2.7, bundling a new libssl12:55
* hazmat upgrades his osx machine to 10.913:13
kadams54guihelp: I haven't seen this kind of failure in a CI build before and I'm not really sure what went wrong: http://ci.jujugui.org:8080/job/juju-gui/1108/console13:29
rick_h_kadams54: looking13:29
rick_h_kadams54: so your failure was in IE https://saucelabs.com/jobs/fc4e2430d0da47ae8d17960df1bd795d to go look at the sauce data13:30
rick_h_kadams54: and the failure was in the notifier widget, which has a giant "FIX ME" because it causes spurious failures so this means just retry. 13:30
kadams54Damn you notifier!13:30
rick_h_:)13:30
rick_h_kadams54: so get that link you have to find the top of that suite run where the link is output13:30
kadams54Yeah, this is one of the tests that consistently fails inconcistently in local dev13:31
kadams54Ah yes, I see the URL now. Thanks.13:31
kadams54How do I trigger a retry? Delete the "Test FAILed" comment?13:31
rick_h_yep, notifier is the one place we still have some timeout based stuff that doesn't clean up well and hits race conditions. To fix it means to really refactor the code heavily so it's a maint. card of work to do13:32
rick_h_kadams54: no, you have to actually login to jenkins and "build with parameters" and put the sha in 13:32
* rick_h_ checks of kadams54 has a jenkins account13:32
rick_h_heh, guess not13:32
rick_h_kadams54: can you please help look at the review backlog and grab one or two please?13:40
kadams54rick_h_: I would if this wasn't my swap day :-)13:41
rick_h_kadams54: oh right, wtf get out of here13:41
kadams54I'm just here to try and get my branch landed…13:41
* rick_h_ boots kadams13:41
kadams54LOL13:41
kadams54Will do.13:41
rick_h_yea, don't worry about it. run away. I'll make sure it gets landed today13:41
rick_h_jcsackett: class based views13:44
* jcsackett blinks13:45
jcsackettdid i miss some chatter?13:45
rick_h_:P13:45
rick_h_jcsackett: no, but it just popped in my head13:45
rick_h_Makyo: hatch halt the line, we need to clear out reviews please before working on anything else. 13:56
rick_h_Makyo: hatch kadams is out today so really need your help to clear the backlog. I'll help out after my call at the top of the hour13:57
hatchrick_h_ sure14:02
frankbanrogpeppe: any idea about why the new suite produces tons of errors? https://code.launchpad.net/~gz/juju-core/logging_dep_fixes/+merge/22122814:16
rogpeppefrankban: almost certainly because it's calling an external command that relies on some environment variables14:18
rogpeppefrankban: for example, $HOME will not be set14:18
frankbanrogpeppe: we are using LoggingCleanupSuite, not IsolationSuite14:18
rogpeppefrankban: ah. hmm.14:18
rogpeppefrankban: can you reproduce the errors yourself?14:18
frankbanrogpeppe: yes14:19
rogpeppefrankban: hmm, seems a little odd to me - i'd try rolling back selectively to see where the problem started14:20
frankbanrogpeppe: oh maybe I've found something14:21
* rick_h_ changes location back to the house. back in a sec14:22
rick_h_ccccccdilrkrtfngcdcubbvfldjujftufgbrdufvirfj14:22
rick_h_ccccccdilrkrgknbcenkijtgtjflrffcrdlivielkfrr14:22
rick_h_bah, yubikey by the headphone jack fail14:23
* rogpeppe authenticates as rick_h14:23
rick_h_bah, was my password so obvious?14:36
hatchlol14:41
rick_h_jujugui call in 214:59
rick_h_bac: around today?15:01
bacy'all sounded amazing on my new noise cancelling earbuds!  so lifelike, except for the lack of ambient noise.15:15
hatchnext time I'll whisper at you so it's like i'm IN your head15:15
rick_h_bac: <3 those. I got those before my last trip and use them a ton now15:15
bachatch: hah.15:16
bacrick_h_: got them mainly for flights.  they have to do all instructions in english and spanish and it goes on forever.15:16
rick_h_bac: yep, why I got them15:17
hatchluca how did you make that Las Vegas story on G+?15:17
lucahatch: it’s made automatically from the pictures I took on my phone.15:18
hatchso you just upload them into the same album and it 'just works' ?15:18
hatchbecause that's really cool!15:19
lucahatch: I’m not sure, I take the pic, it uploads on wifi, I then usually add it to a public album (this one: https://plus.google.com/u/0/photos/113969209489447903536/albums/6006958803479294513) and then it makes it15:19
lucahatch: I think it’s triggered off of a series of photos15:19
hatchcool, must be a new type of auto-awesome15:19
lucahatch: so if you take a lot of a certain period of time it knows your somewhere special15:20
lucayeah15:20
hatchmaybe it also requires the GPS to be on 15:20
hatchso the photos are geo tagged15:20
lucait did one for my holiday in Italy too which is better https://plus.google.com/113969209489447903536/posts/Vr6GLXYdkT615:20
lucayeah, I think it needs GPS15:20
lucaMy italy holiday story is all based around location15:20
hatchcool - so were you biking in that race?15:22
lucahatch: No, thats the Giro, the second biggest professional race.15:22
lucahatch: I followed it for 5 days15:23
lucait’s like the tour de france15:23
hatchohhh cool, I didn't know people did that :)15:23
hatchyou're like a bike groupie :P15:23
lucajust the Italian version of it which has more climbs and worse weather15:23
lucahaha yeah15:23
hatchLooks like it was a great time15:24
hatchand this story is really awesome :)15:24
lucait was great, a lot of fun, a lot of riding and driving15:24
lucaI was really tired after it hehe15:24
lucathe story came out really good15:24
hatchSo how does one follow a bike race? You fly there then bike behind them? 15:25
lucait’s difficult15:25
lucaand complex15:25
lucaI took my bike over15:25
lucaand a rucksake of cycling clothes and a few casual stuff15:25
lucayou can’t cycle the routes because they move too fast15:26
lucaand the roads get cloased off about an hour before they come through15:26
lucausually you get up early in the morning, find a good spot of the course to watch them and spend the time waiting by cycling a part of the course15:26
lucaevery night is a different hotel15:27
hatchthen after they pass drive ahead?15:27
rick_h_hatch: luca yea it goes back through all your photos and builds a story using time/location info15:27
lucayeah, you shoot round to a different part of the course15:27
lucaevery morning you check out of a hotel15:27
lucaso in 5 days we covered 1000’s of km’s, rode about 250km, slept in 4 different hotels and got up every morning at 6:3015:28
hatchheh sounds like an awesome trip :)15:28
hatchand makes for a great story :)15:28
lucahehe15:28
lucathanks15:28
Makyojujugui quick review/qa https://github.com/juju/juju-gui/pull/34715:40
hatchI can do it15:40
hatch#3 for today :P15:40
hatchI feel a little like a gatekeeper15:40
rick_h_today is review day in the UIEng house15:40
hatchMakyo I THINK your CI failures are the spurious ones15:41
MakyoOh, it hadn't loaded the comment.  Let me check.15:41
MakyoI still had the "This can be automatically  merged"15:41
MakyoI'll rerun.15:42
rogpeppefrankban: what commands have you been using to propose a change to github.com/juju/testing ?15:48
rogpeppefrankban: are you using the command line or the web UI?15:48
frankbanrogpeppe: web UI15:48
rogpeppefrankban: ok :-\15:48
rogpeppeanyone know how to create a pull request from the command line?15:52
hatchonnne moreeeee15:53
rogpeppefrankban: also, what command do you use to do the rebase? i always try to read the help for rebase and get totally flummoxed by the maze of flags and possibilities15:53
hatchcode reviews all caught up and shipping the ones that can be shipped15:54
frankbanrogpeppe: rick_h_ can help you more with rebasing, I hate that. I created a script which helps me collapsing changes into a single commit without rebasing, but I am sure it can be considered exotic.15:55
frankbanrogpeppe: anyway, here it is, I called that git-prepare (so that you can run it with "git prepare"): http://pastebin.ubuntu.com/7537428/15:56
rogpeppefrankban: thanks15:57
rogpeppefrankban: wow15:57
frankbanrogpeppe: oh, it uses git sync: http://pastebin.ubuntu.com/7537446/15:58
frankbanrogpeppe: it assumes origin is your own remote (e.g. github.com/rogpeppe/testing) and juju is the central remote (github.com/juju/testing)15:59
rogpeppefrankban: BTW your first sed command could be a little simpler: sed -n 's/^\* //p'15:59
rogpeppefrankban: ah, i've only got one remote - i've been pushing to github.com/juju16:00
rogpeppefrankban: is that considered bad form?16:00
frankbanrogpeppe: yeah, it is really just a quick hack to make me at least understand the process, because I really don't get rebasing16:00
frankbanI have to spend some time learning it I guess16:00
frankbanrogpeppe: it seems a more usual workflow is 1) fork the project on github 2) work on a branch of your fork and 3) propose your branch for merging into the original one (usually develop, but in the juju case I saw master is used)16:02
frankbanrogpeppe: to do that, I basically removed github/juju/testing in my $GOPATH, cloned from my remote, then added the juju remote16:03
rogpeppefrankban: i *think* you should just be able to do git remote add16:03
frankbanrogpeppe: and you can use my "git sync" to sync the two remotes if you like it16:03
rick_h_sorry was afk16:03
rogpepperick_h_: just being my usual git-confused self :-)16:04
frankbanrogpeppe: yeah, but I wanted origin to be my fork, not juju, and I haven't looked at how to rename remotes16:04
rick_h_rogpeppe: heh, it takes some learning for sure16:04
hatchgit makes a lot of sense once you understand wtf is going on under the hood :D16:04
rick_h_rogpeppe: we've got a workflow in the gui hacking doc to help. It has some helpful aliases, process, etc16:05
rick_h_https://github.com/juju/juju-gui/blob/develop/HACKING.rst#typical-github-workflow16:05
hatchof which, I use none because I'm a masochist 16:05
frankbanhatch: other versioning systems make a lot of sense even before understanding C16:05
rick_h_rogpeppe: it tries to work through setting up the two remotes, sync'ing between them, and keeping your branch up to date16:05
hatchfrankban haha, well git is special :P16:06
hatchMakyo not sure if you noticed - I assigned a card for you under Project 116:06
rogpeppei just wc'd the combined output of all the git help commands. 21480 lines.16:06
hatchtoday is the day I break non-il flag16:06
rogpeppethat's quite something for something that's just a glorified fs16:07
hatchrogpeppe lol!!16:07
hatchrogpeppe actually it's a glorified db16:07
hatch:)16:07
frankbanrogpeppe: this can help: http://git-man-page-generator.lokaltog.net/16:07
hatch^ hahaha 16:07
rick_h_:P16:07
rogpeppefrankban: ha ha. is that markov chained from the git man pages?16:08
rick_h_jcastro: sent a nice github link out16:08
hatchrogpeppe https://www.youtube.com/watch?v=ZDR433b0HJY This video is a great intro to git and actually uses git without any of the git commands to show you how it works16:09
rogpeppehatch: i think part of the problem is that everyone these days seems to learn by example, whereas i think i learn best by understanding the primitives16:10
frankbanrogpeppe: sadly that's absolutely realistic, the real git help is stackoverflow IMHO16:10
hatchrogpeppe then that video is your Yellow Sun16:11
hatchholy crap I'm a nerd16:11
rogpeppehatch: i do actually understand how the *underlying* primitives work (well, mostly) - but the primitives we have to work with are the actual git commands16:11
rogpeppehatch: http://en.wikipedia.org/wiki/Yellow_Sun ?16:12
hatchrogpeppe http://en.wikipedia.org/wiki/Superman16:12
hatch:D16:12
hatchsuperman gets his powers from the yellow sun16:13
rogpeppehatch: ah, ok. i'm not big on comic book fiction16:13
* rogpeppe watches the video16:14
hatchlemme know what you think16:14
hatch:)16:14
Makyohatch, okay, thanks.16:14
hatchluca Paolo sounds like someone who uses Reddit :P16:18
lucahatch: rofl16:18
hatchhaha16:18
hatchlooks good luca 16:25
rogpeppehatch: does the video cover rebasing, BTW?16:25
lucahatch: thanks :)16:25
hatchrogpeppe it has been a long time since I've seen it so I can't remember. 16:26
rogpeppehatch: ok16:26
hatchwas there a certain question you had about rebasing though that I might be able to answer?16:26
* rick_h_ goes for food16:30
hatchMakyo just FYI you'll need the il flag for your branch16:35
hatchit doesn't happen with the inspector on the right16:35
MakyoAlright16:35
rogpeppehatch: specifically, if i want to rebase my commits before upstream merging, what would be the usual options and arguments that i'd need?16:36
hatchok assuming you haven't merged anything into this branch since you branched it from develop(or master or whatever your source is) you will run `git rebase -i HEAD~3` where 3 is the number of commits you want to rebase16:38
hatchsay some new code landed in the source branch that you wanted in yours, ----do not merge that branch in---- run `git rebase develop` where develop is the source branch, this will re play your commits on top of the newly updated branch16:39
rogpeppehatch: what if i *did* merge that branch in? or is that a "you're fucked now, boyo" kind of thing?16:40
hatchwell....not totally....but it's considerably more work :) 16:41
hatchthink of git commits always wanting to lay layers on top, if you smush some in the middle it's going to break your sandwich 16:41
hatch(at least the way we do it)16:41
rogpeppehatch: ok, so in general i should *never* merge trunk when developing a branch?16:42
rogpeppehatch: (unlike standard bzr usage, for example)16:42
hatchcorrect - if you really need code thats landed use `git rebase trunk` and, assuming this branch was branched from trunk, it will update your branch to the latest code in trunk and re apply your commits on top16:43
hatchbtw not everyone agrees with using rebase - the git world is really of two camps, the rebasers, and the not :)16:43
rogpeppehatch: IME, you almost always want code that's landed, because you need to check that your changes work in the context of all the changes that have landed since you started work on the branch16:43
rogpeppehatch: i think given the lack of nested commits, rebase is the only reasonable way16:44
hatchyep, so running `git rebase trunk` will get you that16:44
hatchassuming your branch was branched from trunk16:44
hatchgit rebase <source branch>16:44
rogpeppehatch: so the source branch argument must be a direct ancestor of the current head?16:44
rogpeppehatch: ah, no, i see16:45
hatchno, but to keep a clean commit history it should be16:45
hatchcool16:46
rogpeppehatch: where's the HEAD~3 syntax documented?16:46
hatchhmm, not sure, lemme look16:46
rogpeppehatch: and why do you want to make it interactive (the -i arg) ?16:47
hatchsay you had 5 commits, and you wanted to keep 2 outside ones but smush the other 3, this will let you pick and choose ( you can even re order the commits )16:47
rogpeppeand otherwise it smushes them all down to one, right?16:48
rogpeppehatch: one other thing: does rebase actually commit anything, or do you have to rebase, then commit?16:49
hatchI think you need to use --autosquash to have them automatically smushed16:50
hatchrogpeppe it rewrites history so it will be committed but not pushed - if you have pushed those old commits to a remote repo you'll now have to do `git push -f` as the histories won't line up16:51
rogpeppehatch: ah, so when people say "rebase before proposing", that's what they're implying?16:51
hatchrogpeppe well some people code with checkpoint commits and you should only commit logical commits into the main repo - so they want you to get rid of the checkpoint commits and turn them into 'topic commits'16:52
rogpeppehatch: (--autosquash, that is)16:52
hatchand when I say rebase rewrites history...it only rewrites the high level history so if you screw up you CAN get stuff back http://stackoverflow.com/questions/134882/undoing-a-git-rebase16:52
hatchthat's why you never want to commit passwords/keys etc into GIT because it's VERY hard to truly delete something16:53
rogpeppehatch: ah, i didn't know about the reflog16:54
hatchif you think of git as an api on top of a database you realize that you can do pretty much anything once you figure out how the commands work :)16:57
hatchrogpeppe sorry I can't find where the HEAD~3 syntax is documented. But it means from HEAD move two commits back 16:58
hatchHEAD~1 points to head16:58
hatchweird I know...16:59
rick_h_it's like array negative syntax16:59
rick_h_[-1]16:59
rick_h_is the last char16:59
rick_h_[:-3] is the last three chars or commits in this case16:59
rick_h_inclusive of HEAD16:59
rogpeppehatch: so if i want to do the equivalent of "bzr push", squashing all the commits i've made into one commit message, i look in the log to find out how many commits i've made, count them (call it $n), do "git rebase HEAD~$n", then do "git rebase origin" ?17:00
rogpeppehatch: hmm, but i don't see a place to specify the commit message there17:01
hatchclose but not quite17:01
hatch:)17:01
rogpeppehatch: and "git rebase HEAD~$n" isn't going to do anything, is it?17:01
rogpeppehatch: hence the -i flag17:01
hatch`git checkout <souurce branch>` `git pull <remote> <source branch>`17:02
rogpeppehatch: but i hate interactive commands (i almost never use them) so i'd like to know how to do this non-interactively17:02
hatch`git checkout <branch>`17:02
hatch`git rebase --autosquash <source branch>`17:02
hatchI think it'll take the last commit message17:02
rogpeppehatch: the help for --autosquash says that "This option is only valid when the --interactive option is used."17:03
hatchoh wait17:03
hatchyeah autosquash looks for a special commit message17:03
hatchsorry I use interactive :) one sec lemme look into this17:03
hatchkadams54 hey I had one quick comment on your branch then it's gtg mind checking that out :)17:04
kadams54hatch: Yeah, I actually hopped on to ask you about that. I posted a reply in the PR.17:06
hatchcool looking17:06
hatchdoh I missed that17:07
hatchok you can shippit17:07
hatchthanks17:07
kadams54Great17:07
hatchrogpeppe doesn't look like it's possible without some extra scripting :/ sorry17:07
rogpeppehatch: ok.17:08
hatchrogpeppe I've found that I typically don't want to merge them all into a single commit, but instead to a few 'topic commits' so I've become accustomed to using -i17:09
rogpeppehatch: ok, interesting17:10
rogpeppehatch: can you group all the changes in a particular directory into one commit, for example?17:10
hatchrogpeppe there IS a way to commit only blocks of code but I can't remember how to do that. 17:11
hatchgit has a VERY awesome feature called bisect17:11
rogpeppehatch: ok. so in general you'll be squashing contiguous sequences of commits17:11
rogpeppehatch: bzr has that too17:12
hatchoh? oh cool17:12
rogpeppehatch: (well, it may be quite different i guess)17:12
hatchgit bisect allows you to bounce between commits automatically to test where something broke17:12
rogpeppehatch: for finding the commit that introduced a particular thing17:12
hatchso you want commits which help speed this process up17:12
hatchif you end up with one 2000 line commit, that's not going to be very helpful17:12
hatchor if you end up having to bisect through 100 broken wip commits17:13
rogpeppehatch: the other side of the coin is that it's useful having a commit that corresponds to a given code review17:13
hatchfor that we have PR's and a merge commit17:13
hatchsee https://github.com/juju/juju-gui/commits/develop17:13
rogpeppehatch: i found it incredibly great to be able to look at a given commit in the log and jump to the review to see what was going on in people's minds when they made that commit17:14
hatchyep you can do that with the merge commits17:14
rogpeppehatch: so the log we're seeing on that page isn't the set of commits we'll see in the git log?17:15
hatchrogpeppe that's identical to what's in the git log, but github 'enhances' it to provide links directly to the pr's in question 17:16
rogpeppehatch: so, some of the entries there are of the form "Merge pull request #xxx from..." and some are like "This thing happened". Why the difference, and how can I make my commits look like the latter rather than the former?17:17
hatchhere is the git log and the github output https://www.evernote.com/shard/s219/sh/3878c3c3-b935-41ac-8a88-50f6d5f6a9fc/fe800bfa9baa93a8a89fa6f8886a3c6517:18
hatchalthough I just noticed it's using the wrong email for my git commits :/17:18
hatchanywho - when our CI merges the commits it intentionally creates these merge commits so that you can see what commits line up to the PR's17:19
rogpeppehatch: i don't understand. the merge commits are created by jujugui (which presumably is the CI bot, right?), but how do they match up with the other commits, which seem arbitrarily sequenced in between those commits?17:22
hatchthey are ordered by time17:22
hatchthat's why they don't match17:23
rogpeppehatch: oh, that's a bit weird.17:23
hatchyeah - it makes sense the first time you have to bisect something though :D17:24
hatcheasy to find the commit which caused the problem and then the PR associated with it17:24
rogpeppehatch: so the history there doesn't actually correspond to the commit log history, but to when people actually made those changes?17:24
hatchwell no this is identical to the git log history 17:25
rogpeppehatch: and that's ordered by time?17:25
hatchyeah, but remember that people can rebase things, that's why the timestamps are out of wak17:25
hatchalso - if you rebase incorrectly you can screw up the order too :)17:28
rogpeppehatch: but... this feels really odd to me. so you're saying that the log is essentially arbitrarily order?17:28
rogpeppeordered17:28
hatchthere is no 'order' to a git log, it's essentially a flattened tree17:28
hatchit's not like bzr with revno's17:28
rogpeppehatch: flattened by time-of-commit, right?17:28
hatchit's a little more complicated than that....like flattened by tree position17:29
MakyoHere's a helpful alias for logs, if it helps: lg = log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative17:29
hatchrogpeppe to run that copy and paste this : git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative17:30
hatchMakyo good call, that does make it easier to reason about17:30
rogpeppeMakyo: ah, that --graph is useful, thanks17:31
rogpeppeha, less(1) doesn't seem to display colours17:32
hatchrogpeppe you may also find this interesting http://nvie.com/posts/a-successful-git-branching-model/17:32
rogpeppehatch: so if it's flattened by tree position, then i still don't see how successive commits to the tree can get spread out17:33
hatchyeah I don't truly understand it either17:34
rogpeppehatch: hmm, yeah, it's definitely not strictly time-ordered17:35
hatchno, if you rebase commits they can get assigned old dates for new commits17:35
hatchit's kind of funky17:35
hatchit's like, it's position in the graph 17:35
rogpeppehatch: i was just looking at the dates of the commits in the log17:36
hatchI'm sure if you followed the merge step by step through the graph it would make sense17:36
hatchyeah - if you rebase new commits into old commits, they get assigned the old timestamp not the new one17:38
hatchwhich puts the timestamp and graph position 'out of sync' I suppose17:39
rogpeppei have to say it's not entirely clear which commit corresponds to which pull request (for example, I *presume* that 002d86d094977ff33880a126a46b9876a11534ea corresponds to 90a1f669d8885c5fd0f6c7d9c0c591a749ab6c0a but it's just a guess17:39
rogpeppe)17:39
MakyoHuh, didn't notice it until I went back inside to work on a brighter monitor, but the spinner stays overlaid on the inspector in the instance where the sticky headers used to overlay.  I'll see if I can driveby it.17:39
hatchMakyo really? Darn that didn't happen to me, should be a quick fix :) definitely drivebyable17:40
Makyohatch, yep, I'll take a look.  I've got the slow connection.17:40
rogpeppeperhaps the algorithm is "for a given entry in the log, go forward in the log until you find a jujugui commit with the same author"17:40
hatchrogpeppe that does seem like a reasonable technique :)17:40
rogpeppehatch: but is it actually fail-safe?17:41
hatchMakyo heh :) who knew there was an advantage to slow internet17:41
hatchrogpeppe hopefully you don't need to bisect that often17:41
hatch:)17:41
Makyohatch, yeah :)  Can only see it on the nice screen, it's so close to the background color.17:41
rogpeppehatch: it's not necessarily about bisecting, just finding relevant discussion on changes17:41
hatchohh - yeah we typically don't have that many commits between merges so it's pretty easy to find17:42
hatchYMMV I suppose17:42
rogpeppehatch: BTW, is there an equivalent of "bzr qannotate" (i.e. git blame but with a nice place to view the commits corresponding to different lines of code)?17:43
hatchrogpeppe there IS but I don't know how to get to it, you'll have to hit the googles17:43
* rogpeppe is done for the day, having failed to propose his PR. oh well.17:54
Makyohatch, Going through the DOM, there are two whole inspectors and the charm browser all rendered in the left hand side, and it's just the z-index on the expose button making it show through.18:02
rick_h_the new state stuff isn't cleaning up old views on state change18:02
rick_h_?18:02
hatchMakyo ok thats...od18:03
hatchd18:03
hatchyeah sounds like the inspector isn't being destroyed properly18:03
Makyorick_h_, perhaps.  Still digging to find out what's happening.  z-index seems to be the problem with the spinner overlay, too.18:03
hatchmaybe check _emptySectionA method in browser.js18:03
hatchmoving this search is a headache - it breaks darn near everything heh18:04
hatchI wish the watchers worked in vagrant18:11
hatchCSSSSSSSSSSSS18:13
jcastrohey rick_h_18:14
jcastrosince frankban isn't here:18:15
rick_h_jcastro: howdy otp but what's up? 18:15
hatchuh oh18:15
jcastrojuju git-deploy charms/mysql18:15
jcastro:D18:15
hatchyuss the search widget is placed inside the charmbrowser view now18:18
hatchnow to make it work18:18
hatchpoo premature celebration 18:21
hatchMakyo I need to edit the spinner rendering code in the charmbrowser, mind if I do that driveby?18:28
hatchto avoid conflicts18:28
hatchMakyo https://github.com/hatched/juju-gui/commit/9c03ac923b9e92fa97be1ba1ebe873f6f3fe828e18:32
Makyohatch, go ahead, I'm still digging, haven't gotten that far yet.18:32
Makyo+1 on the change18:32
hatch:) If you have any q's about how that stuff works lemme know, I might have an idea :P18:36
hatchrick_h_ i'm in the hangout whenever19:00
* rick_h_ steps away for a few 19:53
hatchback21:07
hatchMakyo hey, any luck on the dual render front? I'm curious :)21:12
Makyohatch, not yet, I was thinking it was due to reloadInspector adding an event after destroy21:23
MakyoBut that doesn't appear to be it.21:23
hatchMakyo maybe if you put a debugger in the initializer and turned on async tracebacks you'll be able to see when they get called21:24
MakyoAlright, will try that out.21:24
MakyoI just can't tell if it's getting reinitialized or if it's being left in place by destroy21:25
hatchahh - well as I understand it, you cannot re-initialize a Y.Base derrived object21:26
hatchbut that doesn't mean that it's not being destroyed properly or that it's not being double-rendered or something21:26
hatchor double initialized for that metter21:26
Makyohatch, it just looks like a destroy thing, but I can't quite pick it apart, there's a lot of hidden stuff going on.  https://gist.github.com/makyo/ab415ea871490c001b59 seems to fix it, but I'm not sure if it's right or not.21:33
hatchlooking21:34
hatchahhh21:34
hatchok lemme dig into this for a sec21:34
hatchMakyo here is the problem https://github.com/juju/juju-gui/blob/develop/app/views/inspectors/service-inspector.js#L9621:35
hatchI think21:36
MakyoThat's what I was thinking, but debuggers never hit in there.21:36
hatchreally?21:36
hatchhmm wth21:36
MakyoYeah, I have no idea.  It's like that's never called under the il flag.21:36
hatchso removing the inspectors container and detaching it's events solve the issue...21:37
MakyoYeah, which has me wondering if destroy is doing all its supposed to.21:39
hatchwell destroy always leaves the container in the DOM in Y.View's which I think is crazy but it's never been an issue for us before because we always pass one in21:39
MakyoYeah.21:40
hatchdoes this 'hidden' inspector work? 21:40
MakyoI guess I was thinking something else.21:40
MakyoLet me check.21:40
hatchor are the events detached21:40
MakyoThey're still attached.21:42
hatchok so that tells me that the destructor isn't being called at all21:42
hatchhttp://yuilibrary.com/yui/docs/api/files/app_js_view.js.html#l14621:43
MakyoYep.  That's why I was thinking it had been intercepted by something.21:43
hatchif you create a destructor method in the service-inspector.js file and put a debugger in there does it get called?21:45
MakyoYeah21:47
MakyoWait, hmm.21:48
MakyoIt's destroyed properly when you click close, just not when you open a second inspector. 21:48
MakyoI noticed that before, but forgot about the state thing,  Let me see if it's destroyed there.21:48
hatchahah!21:48
Makyohatch, if I changestate to null (thus emptySectionA), then changestate to the new inspector, everything's golden.21:52
MakyoSo I need to see about destroying previous inspectors if the inspector changes state.21:53
MakyoThe only downside is that there's a brief flash of the charm browser21:53
hatchahh that would make sense21:53
hatchyeah21:53
hatchwe can probably write a nicer way to render the new one then destroy the old one later on maybe21:53
MakyoYeah.  For now, let me focus on that.21:54
MakyoThanks for being a sounding board21:54
hatch:-) no problemo21:54
MakyoThree line fix.22:12
hatch:D and 100 line tests22:12
hatchlol22:12
MakyoHalf frustrating, half relieved.22:12
MakyoRight? :{22:12
Makyo:P22:12
hatchyeah I feel your pain22:13
hatchhttps://www.humblebundle.com/books I just bought this....hope they don't -all- suck :D22:13
Makyohatch, https://github.com/juju/juju-gui/pull/349 if you're still around22:41
hatchI am22:42
hatchheh nice 22:42
MakyoI'll take a quick look at getting rid of the flash before EoD22:46
MakyoThanks22:46
Makyo...that was easy.22:48
MakyoWell, quick follow up, then.22:48
hatchhaha that was an easy fix after tracking it down :)22:51
MakyoYeah, exactly.  Destroy wasn't being called because destroy wasn't being called :)22:52
hatchlol22:53
hatchdangit where is Huw I have a CSS organization question22:59
* hatch hopes his ears are burning so he comes online22:59
huwshimiMorning23:00
hatchA HAH there he is23:00
hatchsee I told u Makyo  ^ 23:01
huwshimiUh oh23:01
hatch:P23:01
hatchlol23:01
MakyoHaha23:01
hatchdangit where is Huw I have a CSS organization question23:01
hatch[16:59:20]23:01
hatchhatch23:01
hatchhopes his ears are burning so he comes online23:01
huwshimi:)23:01
hatchhuwshimi see line 10 https://gist.github.com/hatched/0584d2ae4d9726a148e823:02
hatchwhy doesn't this work? 23:02
hatchshouldn't it?23:02
hatchI CAN move it out....it just seems like this should be the right way to do it23:03
hatchor does that syntax only work in SASS and not LESS23:04
huwshimihatch: & equals the parent, so you you're trying to find an element that is '.charm-list .with-home [parent] {...}'. I guess that translates to '.charm-list .with-home .charm-list {...}'23:04
hatchohhh23:05
hatchI se23:05
hatche23:05
hatchdug23:05
hatchduh23:05
hatchthanks for making me look stupid23:05
hatch:P23:05
huwshimihatch: If you're trying to select a .charm-list that has the class .with home you need to do &.with-home23:05
hatchhttps://gist.github.com/1c327a656d3c9af423bb is what I'm trying to do23:06
hatchI was just hoping I could keep it more contained23:06
hatchbecause it effects the charm-list element23:06
huwshimihatch: Oh yeah, I hate having do do that sometimes.23:06
huwshimihatch: No solution that I'm aware of.23:07
hatchdrat, alrighty23:07
hatchhuwshimi lemme know if my comments on your PR's don't make sense23:08
hatchI'll be working a bit past EOD today to make up for some lost time23:08
huwshimihatch: Sure, just taking a look.23:09
huwshimihatch: Makes sense.23:12
hatchsounds good - I have no idea why your other branch wouldn't land....23:12
huwshimihatch: Yeah, strange...23:13
huwshimihatch: How do I bind the moveToken event? Something like this? this.addEvent(this.get('unitTokens').on('moveToken', this._placeServiceUnit, this));23:37
huwshimihatch: Will that bind to new unitTokens?23:37
hatchmoveToken is fired from the token?23:38
Makyohatch, quick driveby on that flash: https://github.com/juju/juju-gui/pull/35023:38
hatchhuwshimi: the tokens set the machine view panel to be a bubblle target so in the machine view panel you would go `this.on('*:moveToken', this._placeServiceUnit, this)`23:39
hatchMakyo nice, I can't QA now because I'm in a middle of some search switchover stuff but I'll try later on23:39
MakyoCool, lemme know.  I'll be around23:40
huwshimihatch: Great, that works. Did you want to review this last set of changes (I know there's a +1 already)?23:41
huwshimi(haven't pushed up yet)23:42
hatchsure I'll take a peek once you push23:42
hatchI'm sure it's good :)23:42
huwshimihatch: One sec, I've broken a test.23:44
hatchnp23:47
huwshimihatch: I'll let you know once phantomjs stopps crashing23:48
huwshimistops23:48
hatchoh...good luck, I have to run my tests in the browser because it crashes 100% of the time for me23:49
hatchMakyo I've queued you up another card - this one's going to take some architecture planning :)23:53
MakyoEeeexcellent.23:53
hatchwe should have a pre-imp whenever you want to get started on it23:53
huwshimihatch: Changes are up.23:54
hatchcool I'll take a look23:54
MakyoTomorrow, hatch. Dinner time23:54
hatchyeah definitely :)23:54
hatchhuwshimi looks good 23:55
hatchhuwshimi happy with where the branch ended up?23:55
huwshimihatch: Yeah, it was a bigger branch than I thought it would be, but it seems OK23:56
hatchyeah almost 1000 line diff haha23:57
huwshimihatch: There's a lot of tests in there...23:58
hatchhaha yes, yes there are23:59

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