[01:05] huwshimi I replied to your comments [01:05] I'll take a look [01:06] hatch: Thanks for the heads up on my card. I got it to happen once or twice but then had a hell of a time reproducing. Now I know why :-) I suspect the first few times were due to cached JS. [01:08] kadams54 yeah :) [01:09] huwshimi thanks, so pending that other issue it qa'd ok? [01:09] I'll take a look into it, but because it's pre-existing I might push it to a follow-up [01:10] hatch: Yep. [01:10] thanks, can you +1 it :) [01:11] hatch: Done. [01:11] thanks! [01:11] kadams54 I don't suppose you're up for some late night reviewing? :D [01:11] hatch: sure! [01:12] sweet https://github.com/juju/juju-gui/pull/353 [01:12] thanks [01:12] it's big....:) [01:14] huwshimi I made a card for the double render issue, fyi [01:14] hatch: so why the extension for the search widget? [01:15] kadams54 because the charmbrowser view was getting too big [01:15] and all of those methods were directly related to the widget [01:15] so it was a nice place to split it up [01:16] Why go with plain prototypical object in the extension, instead of Y.Base.create()? [01:16] that's all extensions are [01:16] they are simply a collection of methods which get mixed in [01:17] anything that's Y.Base.create'd is actually a subclass [01:17] think of extensions as mixins [01:17] (y) [01:18] it's a really confusing name, extension [01:18] I like what I see so far [01:18] eggcellent [01:18] Much cleaner [01:18] good thing I've used skype so I know what (y) is :D [01:19] yeah I think this refactoring turned out pretty well [01:21] Review looks good. I'm assuming you also want the snot QA'd out of it? [01:21] that would be nice :) huwshimi found an issue but it was pre-existing so keep that one in mind [01:22] I've got to run for supper, I'll bbl, thanks again for the reviews [01:23] np [07:10] mornin' all === lazyPower is now known as lazypower-travel [09:18] morning fwereade and rogpeppe: I updated https://codereview.appspot.com/92700044, do you want to take another quick look before I approve the branch? [09:18] frankban: looking [09:18] thanks [09:22] frankban: couldn't echoCommand and echoScript still be constants? [09:25] rogpeppe: like const echoCommand = "/bin/echo" [09:25] s.echoCommand = echoCommand [09:25] ? [09:25] frankban: is there a need for SSHCommandSuite to have either of those fields? [09:27] frankban: i'm thinking: const (echoCommand = "/bin/echo"; echoScript = "#!/bin/sh\n"+echoCommand+" $0 \"$@\" | /usr/bin/tee $0.args") [09:28] frankban: it's pretty trivial, but i had to look to see if the fields were variable (included a c.MkDir for example) or not. [09:28] frankban, rogpeppe: yeah, global constants don't bother me the way global vars do [09:28] rogpeppe: ok [09:28] frankban, LGTM anyway [09:29] frankban: LGTM too. feel free to change that or not. [09:34] rogpeppe: updated. rogpeppe, fwereade: thank you both for the reviews [09:42] fwereade: filed 1324841 [09:42] bug 1324841 [09:42] <_mup_> Bug #1324841: Improve isolation in utils/file_test.go [11:15] * frankban lunches [11:15] afk [11:31] back === jcastro_ is now known as jcastro [13:48] Morning all. guihelp: Need a review and QA on https://github.com/juju/juju-gui/pull/354 [13:50] kadams54: on it [13:50] frankban: Thanks! [14:11] kadams54: is it expected that each time I close the inspector the search results are reloaded? [14:12] Right now it gets destroyed and re-rendered, so yes. [14:12] Might be worth talking about whether to just hide/re-show it. [14:16] kadams54: uhm, is it difficult to do that? reloading remote contents does not seem so good to me, also from a UX perspective [14:19] Not sure how easy/hard, but it seems like a new card to me, since it would be a significant change to the current behavior. This card is just about fixing the current behavior. [14:21] The behavior around these will change significantly once hatch's branch lands, so I'm trying to avoid changing too much in this card. [14:22] kadams54: +1 [14:23] I'll circle back after hatch's branch is merged and if the content is still re-loading I'll make a card to be more efficient. [14:23] kadams54: sounds good, review done, could you please create a bug/card for this issue? [14:23] Will do. [14:23] thanks [14:23] Thanks! [14:29] hatch: when you get a chance, I'd like to chat quickly about stubbed methods in PR#352… [14:30] frankban kadams54 The caching branch that Makyo is working on right now fixes that issue [14:30] right now there is 0 caching [14:30] hatch: good to hear :-) [14:31] kadams54 replied to comments in #353 [14:31] let me know if I cna land it [14:32] looking at #352 [14:33] hatch: go ahead and land it. If you want, I can look into the CSS issue so you can focus on removing the il flag. [14:33] sounds good, I need a +1 and a QA OK in the PR :) [14:34] Yup, it's in there. [14:34] Though I just noticed that the merge build is failing? [14:35] replied to #352 [14:35] looking [14:36] looks like an IE10 failure, I'll have to pop into that [14:39] * kadams54 sees much pain in hatch's future. [14:40] we'll see, we'll see [14:48] ugh it's a dependency issue.....stupid YUI [14:51] jujugui stand up in 8:16 [14:51] hah [14:51] :14 [14:52] :12 [14:52] ...we need a really annoying IRC bot that does a countdown in the last 10 seconds. [14:52] "need" may be the wrong word there. :p [14:53] Hahah [14:58] jujugui call in 2 [15:00] jujugui call now [15:01] frankban rogpeppe [15:02] hmm, i seem to be in the wrong hangout [15:02] rogpeppe: https://plus.google.com/hangouts/_/calendar/cmljay5oYXJkaW5nQGNhbm9uaWNhbC5jb20.t3m5giuddiv9epub48d9skdaso [15:02] i'm here: https://plus.google.com/hangouts/_/canonical.com/daily-standup?authuser=1 [15:02] frankban: ok [15:17] jcsacket: can you not bookmark https://plus.google.com/hangouts/_/canonical.com/daily-standup ? (not trying to beat this to death, just curious what you meant.) [15:18] jcsackett: ^^ oops i somehow misspelled your nick [15:19] Makyo: did the candidate submit the homework in advance? i don't see it. [15:20] yay no failures in IE10 locally [15:20] brb going to get coffee [15:20] bac, not that I can see. I'm assuming we'll get that first thing. [15:21] * bac wants to ask him Big Green Egg bbq questions [15:27] rogpeppe: could you please merge trunk and repropose? https://codereview.appspot.com/99670045/diff/20001/testing/testbase/log.go?context=10&column_width=80 imports from github testing/logging which is confusing now [15:27] frankban: ah, ok, will do [15:28] thanks [15:33] bak [15:34] once upon a time, i had a computer that was able to multi-task without preventing me from using the mouse [15:35] rogpeppe haha [15:42] when launchpad sends updates from bugs I wish it included a link to the bug [15:44] it's aaaalll brooken [15:47] well I hope those IE failures in the CI don't show up again because I coudln't reproduce them locally [15:51] kadams54 you can't work on your current card until my branch lands :D [15:52] sorry I forgot to put the blocked symbol on it :) [15:58] and the IE tests are starting [15:58] * hatch crosses fingers [16:04] looks like it passed them [16:08] bac: if i bookmark it, and then go to it, it won't let me in. [16:08] even if i switch authuser [16:10] I -always- have to click the link in the calendar [16:10] else it won't let me in [16:12] so it's been 45 minutes since I bought my coffee and I can finally drink it....I think they make it too hot [16:14] doh! loggo doesn't print anything below warning level by default [16:15] rogpeppe: review done [16:15] frankban: thanks! [16:22] hmm, what's the equivalent of "bzr revert" in git? [16:22] hatch: % [16:22] ^ [16:23] * hatch looks up bzr revert [16:23] hmm, i guess "git stash; git drop" will do it [16:23] well, do you want to keep the changes? [16:23] or throw them away [16:23] hatch: throw 'em away [16:23] `git reset --hard HEAD` [16:23] I'm guessing: "git reset --soft HEAD^" if you want to keep the changes, "git reset --hard HEAD^" [16:23] :) [16:23] HEAD goes the latest [16:23] HEAD^ goes back one commit [16:23] I never thought of using stash though...that's a cool technique [16:24] HEAD^^ back two commits [16:24] I prefer the ~ syntax :) [16:24] hatch: it's less characters to type :-) [16:24] haha [16:25] out for a bit [16:26] jujugui Those participating in the Thibaut Colar interview call in 5 [16:26] 4 [16:26] 321 go [16:26] ! [16:30] Makyo: attempting to join. [17:10] il runs deep with this one [17:36] jujugui https://github.com/juju/juju-gui/pull/355 quick review, but would like two QAs just to be sure. [17:40] Makyo did you forget a file or something? [17:40] I don't see it cancelling the in flight request [17:41] hatch, the problem was that even though it was calling the success callback with the cached data, it was still returning a new request, which was then failing. Returning undefined got rid of that problem. [17:41] ohh ok [17:41] So, not seeing a request to cancel, that becomes a noop. [17:42] sounds good [17:50] Makyo couple qa issues https://github.com/juju/juju-gui/pull/355 [17:50] Cool, thanks, will take a look. [17:56] * rogpeppe is done for the week [17:57] happy weekends all [17:59] enjoy! [18:04] I should have known that after deleting all those loc's the app wouldn't run [18:04] :P [18:05] one line fix [18:05] awww yeah [18:05] go url flags! [18:07] oh boo, we were relying on old code and didn't know it [18:07] down with url flags! [19:20] jujugui anyone available to help me qa this remove il branch before I spend 100h fixing the tests :) [20:16] kadams54 my branch landed so you can style away [20:16] also I noticed the category listing also looks 'off' [20:16] I noticed, thanks :-) [20:27] kadams54 I've removed/modified some css in my il branch https://github.com/hatched/juju-gui/commit/1ffead59a0eea58f915f3a6e3b8b6629752e807c just so we can try and avoid conflicts, (if there are going to be any) [20:27] OK [20:27] Good to know. Still tracking down the problem on my side, so not sure yet what I'll need to change. [20:29] css is such a mess of a language [20:30] I hope one of these days we can switch to sass from less [20:30] it'll help a bit I think [20:46] hatch: how do you think it'll help? [20:46] guihelp: Very small, CSS-only PR that needs review/QA: https://github.com/juju/juju-gui/pull/356 [20:46] well first, I think the less syntax is a little ridiculous so it'll be nicer to read [20:47] sass also provides much nicer functions and the like to work with [20:47] which will help with some of our nested modules stuff [20:47] We used sass on my previous job; my experience with less started on my first day here :-) [20:48] on the top level it's pretty much identical, it's when you really get into it that sass starts to shine [20:48] I'll review/qa your branch [20:50] kadams54 review and qa done [20:54] hatch: FYI, I tried to create a higher specific rule, but !important worked out to be the simpler solution. [20:54] I can switch back if that's the standard. [20:54] I changed the padding because it broke the inside of the list + I don't see the same breakage that you're seeing. [20:55] kadams54 well !important is considered bad practice because it's a smell that the specificity of something else is incorrect [20:55] Yeah, I understand that [20:55] I just take what I consider to be a pragmatic approach [20:55] I'll try to fix specificity as a first resort, but I'm not a zealot about avoiding !important either. [20:56] I am because it's almost impossible to override later, you end up with higher specific !importants hah [20:56] hmm you're not seeing that spacing issue....what browser are you in? [20:57] Chrome Canary [20:57] This is what I see: http://cl.ly/image/0I2K3Y1s0r1i [20:57] ok can you try stable? [20:58] interesting, even in FF I get the spacing issue [20:58] safari too [20:58] Spacing looks good to me in all browsers [20:58] FF, Safari, Chrome [20:58] weird.... [20:59] and the bottom border is across the whole sidebar? [20:59] doesn't stop 30px away? [21:01] Yes, bottom border is across the whole thing and the searchbox is nicely centered in the column [21:01] well then.... [21:01] That's in private mode across all those browsers, which should eliminate caching as a suspect. [21:01] lemme check in Ubuntu [21:01] and IE [21:02] gimme a few [21:02] Yup [21:02] FYI, I pushed a switch to a higher specificity rule, so you may want to re-pull [21:02] Not a force push yet [21:05] checks out in Ubuntu, checking IE [21:05] I can't believe I need whole separate vm's to try different versions of IE lol [21:06] It's integrated with the OS! [21:06] :-) [21:09] ok so it's showing off for me in Win 8 IE 10, OSX Chrome 35.0, OSX Safari 7.0.4, OSX FF 29.0.1 (only the bottom border) [21:09] hmm [21:11] kadams54 do you have an ubuntu or windows vm? [21:11] Yes [21:11] mind giving those a check [21:11] I can't seem to figure out why we would be getting different results from the same codebase heh [21:18] Looks fine to me in all. [21:19] Tried FF and Chrome in Ubuntu, FF, Chrome, and IE in Win7 [21:19] hmm ok we need a tie breaker [21:19] jujugui anyone else in atm? [21:19] FYI, I'm going to be traveling in 10. [21:20] yeah np - I'm assuming you're working off the latest develop as well? [21:20] My vote is shipit [21:20] Even if it is broken, it's less broken than it currently is :-) [21:20] Yup [21:20] Just double checked [21:20] Also a make clean [21:22] ok I'm trying from a fresh clone - will see if it gets better [21:23] kadams54 ok question, what is the width of the #bws-sidebar element? and the input[name=bws-input][type=search] [21:24] 198 for the input [21:25] 290 for the sidebar [21:25] input: 233px sidebar: 291px [21:25] :/ [21:25] wth is going on!!!!! [21:25] lol [21:26]

is 250px with 20px padding all around, which is what the total width of the search widget needs to be. It's what the padding was in the old world. [21:27] yeah it's 233px wide here [21:27] er no [21:27] it's 273px here [21:27] including the padding [21:27] 233px is the 'computed' width [21:29] if I set the width on .search-widget to 250px then it looks good [21:29] but why.... [21:29] whyyyyyyyy [21:29] There should be no width set on it at all [21:29] yeah and there isn't [21:29] Or rather, width: auto [21:29] Your sidebar width is 1px off, but that's not enough to worry about [21:30] ahh I think I found it [21:30] Do you have the mv flag enabled? [21:30] nope [21:30] Hmm, doesn't seem to make a diff for me. [21:30] the search result list has an inline style on it for 233px [21:31] Seems like you're triggering some logic that sets that, while I'm not. [21:31] yeah - I don't even know what that could be [21:32] well lets wait for a third party here - if that comes back ok then we will just shippit [21:32] it's entirely possible my clone is borked somehow [21:33] I'm going to try once more [21:33] for good luck [21:33] Heh [21:34] nothin [21:34] damn [21:35] FOUND IT [21:35] WOOOO [21:36] What was it? [21:37] deleting this rule fixes it https://github.com/juju/juju-gui/blob/develop/lib/views/browser/main.less#L59 [21:38] it overrides https://github.com/juju/juju-gui/blob/develop/lib/views/browser/bws-searchbox.less#L6 which must have been put there for a reason [21:38] That makes me very nervous [21:38] yeah, right? why would the width be specified in two different places, one of which works.....the other doesn't (for me anyways) [21:39] Giving it (deleting) a whirl to see how it works for me [21:39] Looks good. [21:39] Let's roll with it and hold our breath ;-) [21:40] haha sounds good, want to push it, I'll qa it again and then shippit [21:40] this is why I hate css [21:40] :P [21:40] I suppose it's not entirely the languages fault [21:40] haha [21:40] It's pushed [21:40] Once you +1, I'll rebase and squash [21:41] +1'd! [21:43] OK, shipped [21:43] have a good weekend jujugui-peeps [21:43] bac: you too [21:43] And with that, I'm out [21:44] I'll be back on tonight [21:44] cyall === hatch__ is now known as hatch