=== kadams54 is now known as kadams54-away | ||
=== urulama__ is now known as urulama | ||
=== urulama is now known as urulama__|home| | ||
=== urulama__|home| is now known as urulama | ||
kadams54 | hatch: https://github.com/kadams54/juju-gui/commit/252f24ad38649027e64aff7e2252968a0b37fabc#diff-8d19084702f50f400641361a9cb78591 | 14:27 |
---|---|---|
kadams54 | hatch: tests for setMVVisibility | 14:27 |
hatch | hmm is that enough? | 14:28 |
hatch | there are 4 conditionals in the code so shouldn't there be a test for each path | 14:29 |
hatch | it's definitly missing adding a key to the _highlightedServices prop | 14:30 |
hatch | does one of the tests test the !machine.units story? (sorry I'm pre coffee but I don't see it) | 14:32 |
hatch | speaking of which.....running real quick to grab a coffee :) | 14:33 |
hatch | 1H 9M until I can buy linkin park tickets - I may miss our standup :P | 14:51 |
hatch | the ticketmaster experience *refresh* *cuss* *refresh* *cuss* *get seat assignment* *website crash* *cuss* *refresh* *sold out* *wait 2 minutes* *refresh* *get same seat assignment* *cuss* | 14:53 |
rick_h_ | surely something better to do with your time :P | 14:54 |
rick_h_ | hatch: no link for the branch you've got in review | 14:54 |
hatch | nah I almost booked a half day for this | 14:54 |
hatch | :P jk | 14:54 |
hatch | https://github.com/juju/juju-gui/pull/648 | 14:55 |
hatch | that one? | 14:55 |
rick_h_ | hatch: yea, got it thanks | 14:55 |
hatch | rick_h_: that thing you tweeted about would make for a cool charm for people who wanted to host it in their own env | 15:00 |
hatch | jujugui need one more review https://github.com/juju/juju-gui/pull/648 | 15:00 |
kadams54 | hatch: looking | 15:02 |
rick_h_ | hatch: right, like us or other properties in canonical | 15:02 |
rick_h_ | hatch: I want to look more but ran across it in my rss feed last night | 15:02 |
hatch | it's a pretty cool idea if it works :) | 15:02 |
rick_h_ | yea | 15:03 |
hatch | need actions so people can click 'update dictionary' :) | 15:03 |
rick_h_ | heh yea | 15:03 |
hatch | was thinking they should go in the inspector | 15:04 |
hatch | under another tab | 15:04 |
rick_h_ | unit and service details is the first thought, maybe running actions in the inspector for results? | 15:04 |
rick_h_ | because it's unit level as well the inspector is a bit harder | 15:04 |
hatch | there is no service level? | 15:04 |
rick_h_ | there is, service + unit | 15:04 |
hatch | well that's a pretty big oversight no? | 15:05 |
hatch | if I wanted to update the internal dictionary from a remote location for all units I'd have to manually loop through them all? | 15:05 |
kadams54 | hatch: missing key added to _highlightedServices prop is tested implicitly in both "highlights XXX properly" tests. Ditto for !machine.units, since none of the machines setup in beforeEach have units set. | 15:06 |
rick_h_ | huh? you can trigger actions at the service and unit level | 15:06 |
hatch | ohh ok then that's good - sorry I missunderstood | 15:07 |
rick_h_ | hatch: so yes, you can do what you want, and with leader elections there's more power to them as well | 15:07 |
hatch | perfect | 15:07 |
rick_h_ | where the service level trigger can get a single unit nominated to do the action/etc | 15:07 |
hatch | yeah that will be nice - we've needed this functionality for a long time | 15:08 |
rick_h_ | yea | 15:08 |
kadams54 | Alright, this is getting ridiculous. Just had my third merge build fail. Two failures due to NPM errors, one due to a failed notification test. | 15:08 |
hatch | really would like an 'upgrade ghost' button which just goes and pulls the latest version | 15:08 |
hatch | not sure how one would do a rolling update | 15:09 |
hatch | but....yeah | 15:09 |
hatch | :) | 15:09 |
rick_h_ | there was talk in brussels of apis to upgrade single units | 15:09 |
hatch | oh? | 15:10 |
rick_h_ | yea, not sure when but the idea is out there | 15:10 |
hatch | *notes that ticketmaster is down again* | 15:11 |
hatch | well that's good to hear at least | 15:11 |
hatch | kadams54: ok now I see how the tests work | 15:11 |
hatch | could you maybe add some comments to that effect | 15:11 |
hatch | it's not really clear that it's hitting those conditionals | 15:11 |
kadams54 | Will do | 15:13 |
hatch | kadams54: working on this latest branch I've noticed that there is a LOT of noise around the machine and unit events | 15:17 |
hatch | easily a 5x multiplier on thenumber of machines/units per highlight | 15:18 |
kadams54 | Yeah. | 15:18 |
kadams54 | I noticed that we fire off change events for every machine in setMVVisibility | 15:19 |
kadams54 | When in reality we should be checking for machines that actually change their hide attribute and only trigger events for those | 15:19 |
kadams54 | I think re-doing things so that the service token modifies service flags directly, instead of firing an event, will also help quiet things down | 15:20 |
kadams54 | I'll be creating some cards once I get this branch to land | 15:21 |
rick_h_ | kadams54: let's chat and walk through it before hand. I'm nervous of tokens making model changes. | 15:23 |
kadams54 | Sure. I can chat whenever. tldr version is that changing the attributes directly would simplify a great deal. | 15:24 |
hatch | kadams54: did you review 648? | 15:38 |
kadams54 | hatch: No, no I did not. I forgot about it and made a breakfast sandwich instead. Lo siento. | 15:39 |
kadams54 | Looking now. | 15:39 |
hatch | do you? do you feeeeeeel it? | 15:40 |
hatch | I think lo siento literally translated means "feel it" | 15:41 |
hatch | :) | 15:41 |
kadams54 | "Siento" is also the I form of "sentar", which means "to sit" | 15:44 |
kadams54 | hatch: I'm worried about 648. | 15:52 |
hatch | why? | 15:52 |
kadams54 | hatch: the lines that you removed were put in by Makyo as part of the added services work | 15:52 |
hatch | yeah, but they don't fit the spec | 15:52 |
kadams54 | I tested highlighting and showing and they still seem to work, but I'm not entirely sure why. | 15:52 |
hatch | because the code I removed was only for hiding container tokens | 15:53 |
kadams54 | Specifically I'm worried about highlighting | 15:53 |
rick_h_ | right, what lines are we talking about in 648? there's no lines there really, just MV tokens? | 15:53 |
kadams54 | Deleting lines 206-213 in container-token.js | 15:54 |
kadams54 | What's supposed to happen if you have multiple containers on a machine and only one has the service that is highlighted? | 15:55 |
kadams54 | Should the other containers be hidden (as we do with other machines)? | 15:55 |
kadams54 | Or should they be shown, just without their units? | 15:55 |
kadams54 | I don't think the specs we have cover that. | 15:55 |
hatch | kadams54: according to the spec it doesn't say - but it says it should represent the environment | 15:56 |
hatch | so keeping them visible makes sense | 15:56 |
kadams54 | Why? | 15:56 |
rick_h_ | jujugui call in 6 kanban please | 15:56 |
kadams54 | We hide other machines on highlight. | 15:56 |
hatch | this is true | 15:58 |
hatch | but I also think that's weird :) | 15:58 |
rick_h_ | no sense highlighting if the one you're highlighting is 3 scrolls down the page | 15:59 |
kadams54 | I think showing a bunch of machines with nothing on them is pretty useless in highlighting. | 15:59 |
hatch | I would sort them to the top | 15:59 |
kadams54 | Yeah, what rick_h_ said :-) | 15:59 |
rick_h_ | so we either remove machines not highlighted or sort and redorder but now you're doing another evil thing and a pita to do :) | 15:59 |
hatch | because hiding them when trying to be a representation of the hardware doesn't make sense | 15:59 |
rick_h_ | hatch: it's tough because there's not an easy 'grey it out' like in services | 16:02 |
kadams54 | FWIW: both develop and this branch behave the same way when you have multiple containers and only one has a highlighted service: | 16:02 |
rick_h_ | hatch: let's try it one way and get user feedback and update | 16:02 |
rick_h_ | I'm with kadams54 on containers acting like machines for the first pass if it's not in the spec | 16:02 |
kadams54 | They both display all containers and all services on those containers, even the ones that aren't highlighted. | 16:03 |
kadams54 | As long as one service on the machine is the highlighted one. | 16:03 |
hatch | ok it'll be considerably more difficult to do it the other way than I did | 16:06 |
hatch | kadams54: did you want to start on that then while I continue on what I'm on? | 16:07 |
kadams54 | hatch: No, not really. I'd like to talk with rick_h_ about having the tokens update the service flags directly and then work on that :-) | 16:08 |
hatch | well this one is blocking release, that isn't | 16:08 |
kadams54 | Given that this branch doesn't actually change behavior currently on develop wrt containers being hidden on highlight, I'm fine with landing it and will +1 after standup. | 16:09 |
kadams54 | We can then bike shed about correct container behavior separately :-) | 16:09 |
hatch | well it does change it - it makes it so it's not broken lol | 16:09 |
hatch | atm containers with no units are hidden | 16:10 |
kadams54 | Which has nothing to do with highlight. | 16:10 |
kadams54 | So it doesn't change highlight behavior and it DOES fix brokeness elsewhere | 16:10 |
kadams54 | Which is why I'm in favor of landing it. | 16:10 |
hatch | yeah ok cool | 16:11 |
hatch | you make a good point about it matching the machines column | 16:11 |
hatch | see the issue is that there is no way (from within the token) to know if a container should be hidden based on some other service being hidden | 16:12 |
hatch | it just knows that it has no units | 16:12 |
hatch | kadams54: so can you +1 with your comment on the PR so I can land it? | 16:18 |
kadams54 | Yeah. | 16:18 |
hatch | uiteam | 16:31 |
hatch | #first | 16:31 |
hatch | kadams54: when on Monday did you want to start pairing on that stuff? | 17:06 |
kadams54 | Right away? | 17:06 |
hatch | so.....your 9:30 ? | 17:07 |
* rick_h_ goes and takes a deep breath after thinking hatch was going to blame us for using views as widgets :P | 17:07 | |
hatch | lol! | 17:08 |
kadams54 | hatch: whenever you get in. 7:30 seems awfully early to me especially on a Monday. | 17:08 |
hatch | events sent | 17:09 |
hatch | we are only 1H apart now remember :) | 17:09 |
rick_h_ | TZ party! | 17:09 |
kadams54 | Ah yet | 17:09 |
kadams54 | yes | 17:09 |
hatch | rick_h_: I invited you as well but it'll probably be a bunch of fumbling around for the first little while :) | 17:10 |
rick_h_ | uiteam make sure you put the pr urls in the kanban cards please? I've trained myself now but been a few missing today/yesterday | 17:10 |
rick_h_ | hatch: yea, all good. I can let you guys get a plan together without me being nosy :) | 17:10 |
rick_h_ | hatch: I think you know where I'm coming from and have been deeply involved in the lessons of the past so :) | 17:10 |
hatch | yeah we've had some.....discussions.....before | 17:11 |
hatch | :P | 17:11 |
rick_h_ | I think as much as we go back/forth together we've put some good stuff in place. | 17:11 |
hatch | rick_h_: kadams54 so once I finish this current branch I THINK that's all that's necessary for the AS release | 17:11 |
hatch | yup I agree | 17:11 |
rick_h_ | hatch: rgr, will do QA today. Going to deploy to guimaas later on and put it to good use with real env | 17:12 |
rick_h_ | someone say uiteam | 17:12 |
urulama | uiteam | 17:12 |
hatch | uiteam | 17:12 |
* rick_h_ wonders if /reload works or if I have to close/reopen irssi to pick up the higlight | 17:12 | |
rick_h_ | ooh yay ty | 17:12 |
rick_h_ | /reload ftw | 17:12 |
urulama | say ui ... uuuuiiii ... say team ... teeeeeaaaam | 17:12 |
rick_h_ | lol | 17:13 |
hatch | haha | 17:13 |
hatch | I can't find where I change the highlights | 17:13 |
* rick_h_ will like one highlight to rule them all | 17:13 | |
hatch | yeah that was a good idea | 17:13 |
kadams54 | rick_h_, hatch: I'd like to look into getting bundle icons to show up in machine view when you first deploy. https://launchpad.net/bugs/1364493 | 17:14 |
hatch | ok there we go | 17:14 |
mup | Bug #1364493: Machine token charm icons do not show when you add a bundle <juju-gui:Triaged> <https://launchpad.net/bugs/1364493> | 17:14 |
hatch | kadams54: heh that's a deep one | 17:14 |
hatch | pre-imp? | 17:14 |
kadams54 | Yeah, but it annoys the crap out of me. | 17:14 |
rick_h_ | kadams54: hmm, I'd hold off on that. I've got a chunk of work this cycle to get first page loads to work without data and I think that falls into this? | 17:15 |
rick_h_ | kadams54: if there's a temp quick fix (1-2days) ok, but we knowwe've got a bigger problem and it's on the schedule for later this cycle | 17:15 |
kadams54 | Yeah, sure | 17:15 |
rick_h_ | k | 17:15 |
hatch | I'm pretty sure there isn't | 17:15 |
hatch | at least not a clean one | 17:15 |
hatch | someone say ui team :) | 17:16 |
kadams54 | What about this one? https://bugs.launchpad.net/juju-gui/+bug/1379653 | 17:16 |
mup | Bug #1379653: Health bar in the inspector is in the wrong position <juju-gui:Triaged> <https://launchpad.net/bugs/1379653> | 17:16 |
rick_h_ | hatch: yea, that's my fear and why I've got a multi week chunk of work on the schedule for the cycle | 17:16 |
rick_h_ | uiteam | 17:16 |
hatch | woot | 17:16 |
hatch | thanks | 17:16 |
rick_h_ | kadams54: sounds good to me | 17:16 |
hatch | and yeahgood idea with the time block | 17:16 |
kadams54 | +1 | 17:16 |
rick_h_ | kadams54: the notification count one would be good pre-release | 17:16 |
rick_h_ | kadams54: as that's a bit of a regression | 17:16 |
kadams54 | On it | 17:17 |
rick_h_ | kadams54: ty | 17:17 |
* frankban removes juju-gui, jujugui and guihelp from the alerts | 17:17 | |
hatch | frankban: heh I think you were the last remaining guihelp user | 17:18 |
rick_h_ | heh still have that one | 17:18 |
kadams54 | I still have guihelp | 17:18 |
kadams54 | as well as jujugui | 17:18 |
hatch | right, but who used it :) | 17:18 |
kadams54 | I used it all the time! | 17:18 |
kadams54 | Maybe that explains why I could never get anyone to do code reviews… ;-) | 17:19 |
frankban | hatch: I suspected that | 17:19 |
rick_h_ | yea I never got two highlights that hit the same people | 17:19 |
hatch | haha | 17:20 |
rick_h_ | it seemed like giving me two different cell phone numbers to call you on the same phone | 17:20 |
hatch | did you guys ever have 'identicall' where you had multiple numbers that rang differently on the same phone? | 17:21 |
hatch | this would have been back in the late 90s early 2k? | 17:21 |
rick_h_ | yea, when we got our second line when I was a teenager I had that a little bit | 17:21 |
hatch | heh and the caller id separate box | 17:22 |
hatch | ahh people have it too easy today | 17:22 |
hatch | "back when I was your age" | 17:23 |
=== kadams54 is now known as kadams54-away | ||
=== kadams54 is now known as kadams54-away | ||
=== kadams54-away is now known as kadams54 | ||
kadams54 | afk - going to take the dog for a short walk and enjoy some rare sunshine. | 18:45 |
=== kadams54 is now known as kadams54-away | ||
urulama | hatch: if you think yosemite looks bad on non-retina display, imagine how yosemite looks on a hdtm tv! | 19:08 |
urulama | hatch: that thing is horrible! | 19:09 |
hatch | urulama: lol! | 19:17 |
hatch | sigh got to reboot again, vm is being wako | 19:22 |
=== kadams54-away is now known as kadams54 | ||
=== kadams54 is now known as kadams54-away | ||
=== kadams54-away is now known as kadams54 | ||
=== kadams54 is now known as kadams54-away |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!