[00:07] Makyo +1'd [00:17] after moving the search into the new view I can only pass 11% of our tests with 16 failures before it crashes [00:17] lol [00:17] woops [00:17] thats for another day! [00:18] hatch: Nice! [00:19] Yeah I'm pro like that [00:20] here is to hoping your NEW old branch lands heh [00:25] :) [00:26] annnnd I'm out [00:26] have a good one [02:28] wow we have no outstanding PR's go us! lol [02:28] a good day's work :) [02:32] haha yup [02:53] rick_h_ you still around? [02:54] hatch: kinda, packing, loading, playing with toys [02:54] heh, I actually just figured it out [02:54] :) [02:54] woot [02:54] continue playing with toys [02:55] you should be figuring out how to get done for the day and go relax :P [02:55] been there, done that, it's either read this make file or do the dishes [10:45] hi rogpeppe: I am working on migrating to IsolationSuite in utils [10:45] rogpeppe: what do you think about removing the TestPackageDependencies tests from the two or three places in utils/*? [10:55] frankban: (sorry, was afk) [10:55] rogpeppe: np [10:56] frankban: you mean removing the tests entirely? [10:56] frankban: from the github.com/juju/utils repo? [10:58] rogpeppe: well, either removing them or waiting for you to refactor FindJujuCoreImports -> FindImports in github/juju/testing. The former can make sense since we are going to split the package, and we "can" assume juju-core is not imported. Anyway, in utils we have several packages, and only two or three define a dependency test [10:59] frankban: i'm just in the process of proposing the FindImports change [10:59] frankban: (having spend most of the morning trying to come up to speed with git stuff) [10:59] s/spend/spent/ [10:59] rogpeppe: with any success? [11:00] frankban: i think i have a better idea now, but we will see when i try to actually put stuff into practice... [11:00] rogpeppe: heh, I mean, I'd also like to improve my understanding of rebasing stuff [11:00] frankban: i think removing the tests can make sense [11:01] rogpeppe: cool, then in my branch I can just update the testing dependency and refactor the utils test to use the new helper [11:01] frankban: we could possibly have a test at the root of utils that checks that nothing in utils depends on juju-core, i suppose [11:01] frankban: sgtm [11:01] rogpeppe: yeah, that make sense [11:01] frankban: it should probably just be a pre-commit check though (a recursive grep would do it) [11:02] frankban: do we have any precommit checks any more, with the demise of .lbox.check ? [11:02] i guess i mean a pre-propose or pre-merge check [11:03] rogpeppe: yeah, I'll delete the tests. I think we can set up git hooks [11:03] frankban: sounds like a good idea. we should continue to check that code is correctly gofmt'd, for example [11:04] rogpeppe: I am also putting package_test.go files where missing in utils, and replacing juju-core/testing.(Short|Long)Wait with actual values or internal constants when required [11:04] rogpeppe: so that at the end of the process we'll have a self contained utils/* [11:05] frankban: you could have local longWait and shortWait constants [11:05] frankban: great [11:05] rogpeppe: exactly, when they are used once, I'll replace them with actual values, otherwise I used local constants [11:06] frankban: perhaps better to always keep them as local constants so that new tests can use them, but YMMV [11:19] rogpeppe: cool [11:19] * frankban lunches [12:30] frankban: i found this useful w.r.t. rebasing and other stuff: https://www.youtube.com/watch?v=qh-R0-7Ii_U [12:31] rogpeppe: thanks I'll take a look [12:31] frankban: it's the only place i've found so far that talks usefully about git rebase -i [12:32] frankban: (which, BTW, isn't as "interactive" as i feared - it just uses $EDITOR) [12:32] yeah [12:33] rogpeppe: I think FakeHomeSuite should got to github testing as well [12:37] frankban: i'm not sure there - most of FakeHomeSuite is about core-specific stuff [12:39] rogpeppe: FakeJujuHomeSuite is clearly core specific, FakeHomeSuite seems to just create a fake home dir with a .ssh and other stuff [12:40] frankban: ah yes, i didn't notice the distinction there [12:40] rogpeppe: we might need that later, for utils/ssh [12:41] frankban: i'm still not sure [12:41] frankban: it seems like a very specific kind of isolation which is accomplished anyway by clearing the environment, i think [12:42] frankban: i'm open to arguments the other way though [12:48] rogpeppe: looking at ssh test, it seems they need both clearing the env and creating/setting a HOME. I am sure we can decouple them easily from FakeHomeSuite by implementing a suite which embeds IsolationSuite and adds the missing bits. My argument is that this kind of suite could be generic enough to be in github testing, but I am ok either way [12:49] frankban: i *think* i'd prefer something that starts with IsolationSuite and adds the missing bits. what i don't really want to see is every single test adding and removing a bunch of files and directories just because... [12:50] rogpeppe: also consider that for our goal we don't need to move utils/ssh [12:50] frankban: indeed so [13:23] * rogpeppe is still struggling with git [13:23] :-/ [13:23] frankban: how do i create a new repo on github (not through the web UI) [13:23] ? [13:24] frankban: git push says "repository not found" [13:24] rogpeppe: do you need to push a new branch to an existing repo? [13:25] frankban: i've forked juju/testing, and i want to push it to my own github account so i can make a pull request [13:25] frankban: (i pushed it to juju before, but it's been deleted from there, i think) [13:26] s/forked/branched/ [13:26] rogpeppe: try "git push -f origin {featureBranchName}:{featureBranchName}" assuming origin is your fork [13:27] frankban: isn't that pushing it to github.com/juju ? [13:27] frankban: i thought the usual practice was to push it to one's own account and then send a pull req from that [13:27] frankban: ah, i see [13:28] rogpeppe: it depends on your remote name: i set origin to my fork, if origin is trunk, then replace it with the name of your remote [13:28] frankban: so in my case, i've added a new remote [13:28] frankban: i'll try -f [13:28] rogpeppe: e.g. git push -u rogpeppe {featureBranchName}:{featureBranchName} [13:28] rogpeppe: -f should not be reuqired [13:28] frankban: no, still doesn't work [13:29] frankban: perhaps i have to use the web UI to make the repo [13:29] rogpeppe: -u should set the tracking reference, so that next time you should be able to just "git push" [13:29] frankban: (but i was trying to avoid that) [13:29] rogpeppe: ah! you still don't have the repo! so yes, you need to for from github and then push your branches from the command line [13:30] rogpeppe: bah, you need to fork the repo from github. good news is that you need to do that only one time for each project [13:31] rogpeppe: and then push your own branches from the command line, with the command above [13:31] frankban: i did install a github command line client (ghi), but it doesn't seem to work at all [13:34] rogpeppe: FWIW https://developer.github.com/v3/repos/forks/#create-a-fork [13:35] frankban: ah, finally succeeded. part of the problem is i was using github.com/~rogpeppe not github.com/rogpeppe [13:35] heh [13:38] rogpeppe: could you please review https://codereview.appspot.com/92700044 ? [13:39] frankban: i will plough on to try and get this pull request actually submitted first, then i'll look, if that's ok [13:39] rogpeppe: np and thanks [13:39] frankban: do you know how to delete a branch on github? [13:40] rogpeppe: git branch -D $branch [13:40] git push origin :$branch [13:41] rogpeppe: first line to delete it locally, second to delete on github, e.g. "git push rogpeppe :my-old-branch" [13:41] frankban: i don't want to delete it locally, just in github.com/juju/testing [13:41] rogpeppe: ok so the second one: "git push rogpeppe :my-old-branch" [13:42] rogpeppe: push takes the remote name and then a pair of local:remote branches. so you are basically saying push nothing to that remote branch [13:43] frankban: but i still have to delete it locally? [13:43] frankban: i am still confused about the branch name space [13:43] rogpeppe: I don;t think so [13:43] frankban: is it per-repository or per git-database? [13:43] rogpeppe: I think you can just delete the remote one [13:45] frankban: so the ":my-old-branch" is a refspec? [13:45] rogpeppe: yes [13:46] rogpeppe: so it is source-ref:destination-ref [13:46] frankban: yeah, i was just reading the manual [13:51] frankban: now i'm failing dismally to create a pull req [13:51] frankban: i'm in "create a pull request", which wants a repo to compare against, but it's not clear how to refer to the original repo [13:51] s/repo to compare/branch to compare/ [13:53] it doesn't seem to let me enter a branch SHA hash [13:53] or to select juju/testing [13:53] * rogpeppe feels inadequate [13:53] rogpeppe: uhm... when I create a pull request from github it usually automatically recognizes the destination (I guess using the fork source) [13:54] frankban: perhaps because i didn't use github itself to fork the repo, it won't let me do a pull req [13:54] frankban: i guess there's other metadata that github uses outside the repo itself [13:54] rogpeppe: yes, that can be the case. what did you use? [13:55] frankban: i created a new repo, then pushed my forked branch to it [13:55] rogpeppe select 'compare across forks' [13:55] then select 'edit' again [13:56] then in the final dropdown select your branch [13:56] it's a poor UX [13:56] hatch: i did that, but i don't see an "edit" button [13:56] on the far right? [13:56] hatch: nope [13:56] hatch: (just searched for the text "edit" and no match) [13:57] hatch: i see four popups ("base fork", "base", "head fork" and "compare") [14:02] moved discussion... [14:31] frankban: finally: https://github.com/juju/testing/pull/6 [14:31] hatch: does that look like a plausible pull request? [14:32] looking [14:32] rogpeppe: you'll rebase it later, right? [14:32] frankban: yeah [14:33] rogpeppe so with these commits i'd probably rebase them all into 1 [14:33] but other than that [14:33] hatch: ok [14:34] the rational is that formatting isn't really a commit anyone cares about - and if someone was bisecting through a one line comment change that's kind of irritating :) [14:49] frankban: since imports.go is essentially moved from juju-core, i thought it should probably keep the same copyright year. [14:50] rogpeppe: ok [14:50] jujugui call in 10 [14:50] kanban it up [14:57] rogpeppe: :+1: == LGTM (at least that's what we do for the gui) [14:59] jujugui call now [15:00] kadams54 call [15:00] Working on it [15:11] made the news http://thevarguy.com/ubuntu/052814/canonical-designers-work-mobile-friendly-ubuntu-cloud-tool [15:15] hi frankban, proposal up at https://codereview.appspot.com/102870043 -- i have not QA'ed it on OS X yet as I am working through and documenting the installation of dependencies before i can get quickstart built. [15:15] bac: cool I'll take a look [15:15] ty [15:16] jcsackett: maybe you can get paul and deryck to answer your convoy questions via twitter [15:16] bac: it's a thought. [15:16] looks like it was rick and ian doing the work. [15:17] why does the phone hangout app make joining hangouts so difficult? [15:18] hazmat wow I had no idea that was public [15:31] jcsackett every time we bring something from LP it seems it wasn't written to be portable heh - (the textarea resziser) :P [15:47] hatch: would like to chat about state when you have a few minutes [15:49] kadams54 sure, couple mins [15:53] kadams54 ok ready, link me [15:54] https://plus.google.com/hangouts/_/gxykkr3joazsb2prf5fdw2j5bea [15:54] party is over? [15:54] https://plus.google.com/hangouts/_/gxykkr3joazsb2prf5fdw2j5bea?authuser=2&hl=en [15:54] no luck [15:55] https://plus.google.com/hangouts/_/gwks23a34eatqqhdu46mbaeqiia?hl=en [15:55] try that [16:04] evening anthonydillon [16:08] luca thx for the new design - but is the 'Local charm' heading supposed to not be centred vertically in the 'header area' [16:10] hatch: now that you mention it, it does look weird, I’ll ask Spencer to have a look and get back to you [16:10] sounds good :) [16:11] luca it would be rockin if you could also get him to add dimensions/hex codes ect [16:11] I know it's more work, but saves us from guessing :) [16:11] hatch: thats fine, he has a tool that does that for him so wont take long [16:11] oh nice, now I won't accept anything but :P [16:14] bac: review done [16:15] hatch: lol [16:16] haha [16:17] rogpeppe: I had a similar idea re ExcludeEnvVars, that sounds good. I'd be inclined to add that later, in another branch [16:18] frankban: sgtm [16:19] data, err was tricky. data was defined globally in another test file, which was not even executed. When I fixed the test, I saw data was not the expected value :-/ [16:19] rogpeppe: ^^ [16:20] frankban: ha [16:20] frankban: so was the other data value actually used? [16:20] rogpeppe: yes, causing the test to fail [16:21] rogpeppe: it seems to me a good practice for tests is to not pollute the global namespace, especially with generic names like "data", correct? [16:22] frankban: *definitely* with generic names like data [16:22] frankban: i think it's fine for tests to create globally named tables when they are named explicitly after the tests [16:23] rogpeppe: yeah [16:23] frankban: and i actually think it can aid readability [16:23] frankban: although this is a topic on which there has been some debate :-) [16:23] oh man I love this new state code [16:24] :-) [16:24] guihelp: https://github.com/juju/juju-gui/pull/352 is ready for review and QA [16:24] SEE!!! [16:24] that's why it's so awesome [16:24] lol [16:25] sorry I'm in the middle of some stuff if someone else could pick up that review [16:33] frankban: BTW, do we have a LGTM convention for replies to PR's on github? or is that thumbs-up symbol automatically created from the word "LGTM" ? [16:34] rogpeppe: thumbs up is :+1: , and that's just our convention for the gui development. [16:34] rogpeppe: http://www.emoji-cheat-sheet.com/ [16:35] * rogpeppe tries to see a thumbs up in those characters [16:35] rogpeppe: in the GUI, having CI connected to github,we also use :shipit: to automatically start the CI/landing process [16:35] frankban: do you know what the convention is in github.com/juju? [16:36] rogpeppe: no [16:36] it would be nice if they were the same convention :) [16:41] hatch, Hey, hows it going? [16:41] motoring along [16:42] crazy storms here lately, hoping I don't have to deal with water in the house [16:42] and yourself? [16:42] it's rained 6" in 24H lol [17:06] ha, i *think* i've done my first successful rebase and push [17:07] the review conversation is lost though. i guess that's inevitable, though sad. [17:07] i think i'll try to include a link to the pull request in the commit message in future [17:08] hatch, frankban: could you sanity check this repo please, just to make sure i haven't been stupid? https://github.com/juju/testing [17:09] sure [17:09] you guys don't have a CI so you clicked the big green button? [17:09] ^ rogpeppe [17:09] hatch: that's right. actually i just did "git push" but same difference, i presume [17:10] oh ok, no you did it wrong [17:10] :D [17:10] hatch: no CI there [17:10] rogpeppe https://github.com/juju/testing/pull/6 scroll to the bottom [17:10] see the big green button [17:10] hatch, rogpeppe: yeah there is no "Merge pull request" message in the commit history [17:10] you were supposed to rebase, push back to the pr branch, then push the big green button [17:11] hatch: but if i did that, all the conversation on the PR would be lost, wouldn't it? [17:11] this isn't horrible, but now you don't have reference to the PR in the commit history [17:11] rogpeppe no, it's just hidden [17:11] hatch: but by rebasing, won't i have lost the items from the commit history? [17:11] rogpeppe see this one https://github.com/juju/juju-gui/pull/341 [17:12] hatch: so presumably after rebasing i'd need to do push --force onto my branch, right? [17:12] correct [17:12] git push -f [17:13] rogpeppe it's ok, you'll get it :) [17:13] so many ways to get it wrong. so few ways to get it right :-) [17:13] lol truth [17:13] we tried to find a way to disable the wrong ways [17:14] rogpeppe, hatch: we definitly need to add CI and :shippit: to github/juju stuff [17:14] but it looks like you just have to be careful [17:14] frankban agreed [17:14] maybe after this cycle.....*snicker* [17:14] hatch: so the reason the comments say "outdated diff" is because the commit has been lost by rebasing, right? [17:15] hatch: but i'm guessing that github should keep the commits from being GC'd because they've got comments referring to them, yes? [17:15] correct [17:15] they are in the reflog [17:15] so nothing is really 'lost' it's just very well hidden heh [17:16] hatch: the reflog doesn't prevent things from being GC'd [17:16] hatch: after 30 days, *gone* [17:16] hatch: at least that's my current understanding [17:16] oh....well no the comments are always there [17:16] hatch: but can i retrieve the branches they're referring to? [17:16] if you wanted to dig through the reflog [17:17] hatch: for example if i want to see the entire context of someone elses conversation [17:17] it's not like bzr in that they are still first class citizens [17:17] it's quite difficult to get that back [17:17] oh [17:18] hatch: for example, i very often need to look at the entire file to see the context, rather than just a limited window. the standard diff view doesn't seem like it provides that. [17:18] if you're worried about rebasing after reviews you can make logical commits after the fact and if you don't rebase those out, then those commits and comments will still be there [17:18] I typically rebase those out because the original commits are no longer valid without the review changes [17:19] * rogpeppe found the rietveldt model worked pretty well for this stuff. [17:20] i very often would do a diff between different stages of the code review [17:20] rogpeppe the BIG difference now is that we really try and make much smaller commits [17:20] er [17:20] PR's [17:20] when possible [17:21] hatch: we always tried to do that. sometimes it just doesn't work out. [17:21] I've noticed that our PR size has dropped dramatically since switching from bzr to git [17:21] guihelp: I probably missed this in standup, but anyone know what the status is on Huw's "Wire existing containers and machines into the unit token." card? It's in the review lane but I don't see an associated PR. [17:21] kadams54 hmm I wonder if that's one of the ones that landed [17:21] lemme check [17:22] It seems to have landed… there are dropdowns with machines/containers in the unplaced unit now [17:22] rogpeppe so the issue you have is that commits after putting up for review get rebased out without their comments? [17:22] kadams54 yeah I think there was some mixup with the card naming [17:22] you can probably drag that over [17:22] hatch: that's a potential issue raised by others, yes. [17:23] hatch: i don't know if it's actually the case or not. [17:23] hatch: Good. It'll save me from having to come up with an explanation for exceeding max WIP :-) [17:23] kadams54 my explanations are usually "jus tryin to get work done man" [17:23] :P [17:23] hatch: i just want to make sure that a) the commit history looks sane and b) all the conversations are available indefinitely after the merge has been done. [17:23] hatch: should his card go in Daily Accountable or Landing? [17:23] * rogpeppe needs to stop for the day [17:24] g'night all [17:24] rogpeppe tbh it hasn't been an issue with the GUI, but you can play it by ear [17:24] gnt [17:24] kadams54 landing [17:30] hey hatch [17:30] since rick is missing I shall bug you ... [17:30] I have an odd request [17:30] is there a way we can slow do then deployment animation on jujucharms.com? [17:30] to kind of make it not so fast/instant? [17:31] hmm [17:31] no [17:31] well not without some time investment in it [17:32] jcastro you could hook it up to a local env, then it'll be much slower :) [17:33] yeah [17:34] jcastro the issue is that when you're on a fake env it doesn't go through the typical lifecycle stages [17:34] so those would have to be simulated on top of the deployment [17:34] which is not a trivial fix [17:35] well, it doesn't need to be realistic [17:35] just not instant [17:35] like, add a few seconds [17:36] yeah the problem is that just flips a switch to deployed, there is no system in place for simulating the steps [17:39] hatch, got a sec for a call? [17:39] It can wait, too. [17:39] yeah link me [17:40] standup is empty: https://plus.google.com/hangouts/_/canonical.com/daily-standup?authuser=1 [19:50] oh will these tests ever end!!!! [20:05] guihelp: looking for a review and QA on https://github.com/juju/juju-gui/pull/352 [20:46] kadams54 I'll get to it before EOD if noone else does, I'm still powering on these darn tests [20:47] 3 more to write then fixes (I'm sure) [20:47] :-) [21:10] 3 failures [21:10] left [21:21] jujugui looking for two reviews and qa's https://github.com/juju/juju-gui/pull/353 plz and thanks [21:22] kadams54 I can do yours now [21:24] kadams54 done - one comment [21:26] jujugui splitting my day to run down and pick up stuff for the move, will be back on this evening. [21:26] hatch, will likely miss the Australian call; you planning on heading to that? [21:26] yep [22:26] Morning [22:30] hatch: Do you know if the AU call is happening today? [22:30] huwshimi well it's just me [22:30] :) [22:30] I think everyone else has left me [22:30] haha, ok [22:31] I have a branch which you can review and qa though [22:31] hatch: Sure [22:31] https://github.com/juju/juju-gui/pull/353 [22:31] doh! [22:31] I committed stuff I shouldn't have [22:31] fixing [22:32] np [22:33] updated [22:35] I want to get this branch landed when I get in tomorrow so I can start switching over the il flag [22:36] kadams54 the card that you have in starting is already done [22:36] Makyo had fixed it yesterday