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