| frankban | morning rogpeppe: could you please review https://codereview.appspot.com/7644043/ ? | 10:04 |
|---|---|---|
| rogpeppe | frankban: will do, after i've finished the review i'm currently on | 10:05 |
| frankban | thanks rogpeppe | 10:05 |
| rogpeppe | frankban: LGTM | 10:44 |
| frankban | rogpeppe: great, thanks | 10:44 |
| frankban | rogpeppe: I am trying to add retries to setAnnotations, in order to avoid possible race conditions. could you ping me when you have a minute? | 11:59 |
| rogpeppe | frankban: now's good | 12:00 |
| frankban | cool rogpeppe, thanks: this is a prototype, not sure if it makes sense: http://pastebin.ubuntu.com/5633877/ | 12:01 |
| rogpeppe | frankban: looking | 12:02 |
| rogpeppe | frankban: that looks good, although i think i'd use a slightly higher iteration count | 12:04 |
| rogpeppe | frankban: in fact, that makes me think | 12:04 |
| * BradCrittenden reboots | 12:04 | |
| benji | man, lately my email has exceeded even the hights of the Launchpad firehose | 12:04 |
| rogpeppe | frankban: maybe... we should never remove values | 12:04 |
| rogpeppe | frankban: because removed values are the main thing that can cause contention here. | 12:05 |
| rogpeppe | frankban: and, honestly, do we care about a little garbage left over from blank values? | 12:05 |
| rogpeppe | frankban: what do you think? | 12:05 |
| frankban | rogpeppe: contention can be caused also by DocMissing/DocExists interaction, right? | 12:06 |
| rogpeppe | frankban: yes, but that automatically resolves itself after one iteration, because we don't remove the doc when it's empty AFAICS | 12:07 |
| rogpeppe | frankban: but if two things are both trying to set an attribute, one to "x" and the other to "", they can iterate forever AFAICS | 12:07 |
| rogpeppe | frankban: well, i may be exaggerating there - i haven't properly gone through it. | 12:08 |
| frankban | rogpeppe: why do you expect them to iterate? doesn't the second just win? | 12:08 |
| frankban | rogpeppe: i.e. isn't ErrAborted returned only if one of the assertions fail? | 12:09 |
| rogpeppe | frankban: the point is that if we never remove an attribute, then there can *never* be more than two iterations, regardless of what anyone else is doing, AFAICS | 12:09 |
| frankban | rogpeppe: confused, how to calculate the number of required iterations is the first of my questions, the second one being: how am I supposed to test contention. I supposed that, since both updates and removals assert the same thing (DocExist), they would never interact in a way that causes the transaction to abort. | 12:16 |
| rogpeppe | frankban: currently we don't test contention anywhere. i'm not sure how to do so reliably, because it inherently relies on a race, but i think we should definitely think more about that | 12:18 |
| rogpeppe | updates and removals *can* cause the transaction to abort | 12:18 |
| rogpeppe | frankban: because removal removes the doc, and update is checking that the doc exists. | 12:19 |
| frankban | rogpeppe: AFAICT removals removes key/value pairs from the doc, not the doc itself | 12:20 |
| frankban | s/removes/remove | 12:20 |
| rogpeppe | frankban: that's a bloody good point, and one i hadn't realised. | 12:20 |
| rogpeppe | frankban: so... i think all's good | 12:20 |
| frankban | rogpeppe: cool, so, about the number of retries, should I just decrease them to 2? | 12:21 |
| rogpeppe | frankban: yes, i think so, and add a comment why that's the right number | 12:22 |
| frankban | rogpeppe: cool, thanks | 12:22 |
| rogpeppe | frankban: one thing occurs to me: | 12:23 |
| rogpeppe | frankban: could you do the $set and the $unset within the same Op? | 12:23 |
| rogpeppe | frankban: so it would look like: Update: D{{"$unset", toRemove}, {"$set", toUpdate}} | 12:24 |
| frankban | rogpeppe: aha, good point, that way we always have a single op | 12:24 |
| rogpeppe | frankban: yeah | 12:24 |
| frankban | rogpeppe: and I can rename the helper function to setAnnotationsOp. I'll try this | 12:25 |
| rogpeppe | frankban: yeah, sounds good | 12:25 |
| rogpeppe | frankban: one other thing: i'm not sure quite what you're trying to do with hasAnnotations there. perhaps you could explain it? | 12:26 |
| frankban | rogpeppe: eh, that was another question, I was reading the draft from William, and he wrote someting like "if mongodb is already in a state consistent with the transaction having already been run -- which is *always* possible -- you should return immediately without error." | 12:29 |
| frankban | rogpeppe: I am thinking at the case when a client tries to set the same annotations simultaneously two times and the doc is missing | 12:31 |
| rogpeppe | frankban: i think that'll just work. | 12:31 |
| frankban | rogpeppe: yes, I agree, we write two times the same pairs in mongo and that just works. So, no need to avoid the second transaction? | 12:33 |
| rogpeppe | frankban: yeah | 12:34 |
| rogpeppe | frankban: i think there's no need for another round trip | 12:34 |
| rogpeppe | frankban: if the transaction fails the first time, it can only be because the document does not exist, so we run it again, and then we know we're ok | 12:35 |
| frankban | rogpeppe: ok, last thing: should we check that the entity still exists? if, e.g., a service is destroyed, we can have a race (the document no longer exists) | 12:36 |
| rogpeppe | frankban: yes, i think so, for precisely that reason. | 12:36 |
| rogpeppe | frankban: although... | 12:37 |
| rogpeppe | frankban: i don't know if any harm will come if we don't check that | 12:37 |
| rogpeppe | frankban: the only problem might come if a service is destroyed and then immediately recreated with the same name | 12:37 |
| frankban | rogpeppe: in that case, it is possible that we update annotations on the new service, is it so bad? | 12:40 |
| rogpeppe | frankban: well, if someone is constantly adding and removing a service with the same name, we could go more than 2 iterations around the loop... | 12:41 |
| rogpeppe | frankban: i think that in the future, we'll hopefully move towards having a unique id for everything, so this issue will go way | 12:42 |
| rogpeppe | s/way/away/ | 12:42 |
| frankban | rogpeppe: yes | 12:42 |
| rogpeppe | frankban: i think let's not worry about it for now | 12:42 |
| rogpeppe | frankban: though fwereade__ may well call us out on it :-) | 12:42 |
| frankban | rogpeppe: ok | 12:43 |
| rogpeppe | frankban: anyway, it's trivial to fix either way (increase iter count / add DocExists assertion) | 12:43 |
| frankban | rogpeppe: add DocExists assertion? | 12:44 |
| rogpeppe | frankban: currently there's a DocExists assertion for the $set/$unset ops. i'm not sure it's needed. | 12:44 |
| rogpeppe | frankban: if you're changing annotations on a service which has been destroyed, i think it's fine to drop them on the ground. | 12:45 |
| frankban | rogpeppe: I was thinking about: after two retries, if there's still an ErrAborted, we call Annotator(name) to check if the entity exists. If it exists, return ErrExcessiveContention, otherwise, return another error message | 12:48 |
| rogpeppe | frankban: that sounds great. | 12:49 |
| rogpeppe | frankban: except i'd probably use Count rather than calling Annotator | 12:49 |
| frankban | rogpeppe: if this makes sense, then we still need DocExists on updates/removals | 12:50 |
| rogpeppe | frankban: yeah, true. | 12:50 |
| rogpeppe | frankban: sounds good. | 12:50 |
| frankban | rogpeppe: re Count, yes, better | 12:50 |
| frankban | rogpeppe: and the custom error is something like "entity $entityname no longer exists" | 12:51 |
| rogpeppe | frankban: yeah, or actually, just "$entityname no longer exists" | 12:51 |
| rogpeppe | frankban: given that entity names are fairly self-describin | 12:52 |
| rogpeppe | g | 12:52 |
| frankban | rogpeppe: cool | 12:52 |
| * frankban lunches | 13:23 | |
| benji | heh, "a runtime error occured in hr-self-service application" | 14:05 |
| hatch | a runtime error heh - I haven't seen one of those in a long time | 14:15 |
| gary_poster | jujugui call in 1 | 14:28 |
| * benji hopes the Google Talk plugin spirits smile upon him. | 14:29 | |
| gary_poster | rick_h_ starting | 14:30 |
| hatch | hey rick_h_ I saw your tweet (at least I think it was you) I use an aeron char | 14:41 |
| hatch | and I love it | 14:42 |
| hatch | worth every penny | 14:42 |
| rick_h_ | goodspud: cool I see it on my calendar but don't recall seeing hte email. Is there anything I need to prep for it? | 14:42 |
| rick_h_ | hatch: cool, yea I think I'm settling on the ergohuman v2 but want it with the headrest as I'm told it's more useful than you'd think. | 14:42 |
| rick_h_ | but looking for one to order still | 14:43 |
| goodspud | rick_h_, we are wanting to understand what information you need from us to work on the "make it look pretty" side of things (match the visual design) | 14:43 |
| hatch | ahh yeah the headrests can be nice | 14:43 |
| goodspud | rick_h_, and the best format for that information | 14:44 |
| hatch | gary_poster: so I'm going to do the _nsRouter ticket now but then I'm out of thinks on my list - anything specific you need completed or should I Just spin around in my chair and pick one at random? :) | 14:44 |
| rick_h_ | goodspud: ok cool. see you in 15 then | 14:44 |
| gary_poster | hatch, spinning around in your chair until you induce nausea might be fun. | 14:45 |
| hatch | best.....job.....ever! | 14:45 |
| hatch | weeeeee | 14:45 |
| gary_poster | lol. hatch, do you have any kind of tablet? | 14:45 |
| hatch | N7 | 14:45 |
| gary_poster | hatch, maybe a good task for you would be to connect with Makyo about his old touch branch and see what you can do to (a) identify touch issues that need to be addressed, divided by technical issues and design issues; and (b) resolve some of the low hanging fruit. | 14:47 |
| gary_poster | wdyt? | 14:47 |
| hatch | sure thing - I checked it out the other day with hazmat and found some interesting things across platforms and browsers | 14:47 |
| gary_poster | cool, hatch, thanks. I have another task I can put you on next week, if you solve all problems and/or want to switch gears | 14:48 |
| gary_poster | (it would be to contribute to the in-browser-memory juju stuff I've been starting, that you reviewed a branch of yesterday_) | 14:49 |
| hatch | sounds good | 14:49 |
| hazmat | hatch, another datapoint to add works fine on an ipad2/chrome.. current staging doesn't load for me on iphone5 (although it has in the past with more minimal content est) | 14:49 |
| hazmat | set | 14:49 |
| hatch | hazmat: interesting - especially considering ios chrome is just a wrapper around webview heh | 14:50 |
| hazmat | indeed. though this is hardware delta iphone 5 vs ipad2.. | 14:50 |
| hatch | true - do you -ever- get script running to long errors on ios safari? | 14:51 |
| hatch | I have never seen one | 14:51 |
| gary_poster | I've never seen them | 14:51 |
| hatch | so I was thinking maybe it's squashed and just kills it | 14:51 |
| gary_poster | heh maybe so | 14:51 |
| hatch | iunno this is just speculation :) | 14:51 |
| hazmat | hatch, hmm.. actually it just loaded on the iphone5 after a reload (chrome). | 14:52 |
| hazmat | but no.. never seen any error dialogs when it doesn't load | 14:52 |
| hatch | and on my phone it gave me a script running too long error and asked if I wanted to continue - then loaded properly | 14:52 |
| hatch | iunno just an idea | 14:52 |
| hatch | will take some testing | 14:52 |
| hatch | I have an iphone 3g which I can test on too if we want to support that far back | 14:53 |
| gary_poster | tablet chrome is all we want atm. hopefully more later. | 14:54 |
| gary_poster | like second half of year kind of thing | 14:54 |
| hatch | cool - tablet firefox works better right now ;) | 14:54 |
| hatch | actually for the phone it does too | 14:54 |
| gary_poster | hatch, interesting. | 14:54 |
| hatch | I just found it funny because it worked better and was faster and gave a warning that it's not supported hehe | 14:55 |
| gary_poster | :-) | 14:55 |
| gary_poster | we can remove that warning for FF once CI tests are up, IMO | 14:55 |
| hatch | yeah I'd agree - lately it looks like FF has been closer to the spec than chrome | 14:56 |
| hatch | or maybe mozilla just has a better marketing department | 14:56 |
| hatch | :D | 14:56 |
| gary_poster | heh | 14:57 |
| hatch | rick_h_ also if you come across any odd path/route issues plz let me know - I tried to account for all the typical use cases but it's entirely possible that i miss one | 14:59 |
| rick_h_ | hatch: rgr | 14:59 |
| hatch | rick_h_ do you have any idea when you might have a subapp with multiple functional routes? I'd like to demo this namespace stuff but it doesn't really show it's coolness untill there are two functional apps. Do you think in a couple weeks? | 15:06 |
| hatch | no rush or anything i'm just curious | 15:07 |
| frankban | rogpeppe: hi, I have to handle this case: I have a function returning a txn.Op and an error. Sometimes, even without errors, there is no Op to be performed (so a zero value txn.Op is returned). The caller must check if it needs to run the Op. I see 4 possible solutions: 1) check that one field is not the zero value 2) the function returns a slice of operations (even if that slice will contain only zero or one ops) | 15:14 |
| frankban | and the caller check len(ops) 3) the function returns a *txn.Op and the caller check != nil 4) the function returns (op.txn.Op, required bool, error) | 15:14 |
| rogpeppe | frankban: one other possibility: define var errNoOperationRequired = errors.New("no operation required") and check for that explicitly | 15:15 |
| rogpeppe | frankban: when is there no op to be performed? | 15:16 |
| rick_h_ | hatch: well, I've got two views that 'render' right now and this branch I'm trying to get up today. | 15:16 |
| rick_h_ | hatch: so in theory if I bind a click event you can swap between two views this week | 15:16 |
| frankban | rogpeppe: aha! when the document is missing and the request contains only removals | 15:16 |
| hatch | oh very cool - ok cool I'll make a note for next week | 15:17 |
| rick_h_ | gary_poster: is there a staging besides uistage that's got a more recent deploy of trunk? /me isn't sure how staging/etc is working. | 15:17 |
| hatch | I want to demo it for the YUI Round Table | 15:17 |
| rick_h_ | hatch: hah, well....as I just showed goodspud "it ain't pretty" | 15:17 |
| hatch | oh haha | 15:17 |
| gary_poster | rick_h_, uistage used to be updated every half hour from trunk. checking. otherwise, I or you can spin up a charm | 15:18 |
| rogpeppe | frankban: i wonder if you could create the document in that case anyway (with an empty set of annotations) | 15:18 |
| rick_h_ | gary_poster: ah ok. Yea my url/route isn't working on it but it works locally. I figured it was just out of date. Maybe there's a bug I didn't know of in production then | 15:18 |
| rogpeppe | frankban: then we preserve the invariant that if you call SetAnnotations, the document must exist afterwards. | 15:19 |
| rick_h_ | gary_poster: ah nvm, it's got hatch's changes so it's down below | 15:19 |
| gary_poster | rick_h_, oh ok cool | 15:19 |
| rick_h_ | and mousewheel scrolling fooled me | 15:19 |
| hatch | lol | 15:19 |
| rick_h_ | goodspud: so yea, you can see the stuff load under the environment at http://uistage.jujucharms.com:8080/bws/sidebar and http://uistage.jujucharms.com:8080/bws/fullscreen | 15:19 |
| rick_h_ | goodspud: it's just off the page | 15:19 |
| frankban | rogpeppe: it's doable, but do we care about that invariant? | 15:20 |
| rogpeppe | frankban: doesn't it make the code a little simpler? you then don't need to worry about returning no ops | 15:20 |
| rogpeppe | frankban: and though strictly a little less efficient, i can't see that it matters. | 15:21 |
| gary_poster | rick_h_, (it is every half hour still fwiw) | 15:21 |
| rick_h_ | gary_poster: thanks, yep ignore me. it's just working better than I thought :) | 15:22 |
| gary_poster | :-) cool | 15:22 |
| goodspud | rick_h_, cheers buddy | 15:22 |
| frankban | rogpeppe: +1, it's achievable just removing an "if" | 15:23 |
| rogpeppe | frankban: thought so | 15:23 |
| frankban | rogpeppe: thanks. anyway, it's nice to know how to handle similar situations using a custom error | 15:26 |
| rogpeppe | frankban: yeah, it's a useful technique | 15:26 |
| hatch | if anyone has a few minutes to review https://codereview.appspot.com/7946043 trivial attribute namechange | 15:36 |
| benji | hatch: I'll take a look. Is there a card on the board I should tag? | 15:37 |
| hatch | yup in the top row | 15:39 |
| benji | done and done | 15:39 |
| hatch | thank you! | 15:39 |
| benji | I don't like the "LGTM" bit. I need to write up an RFC that codifies +/- 1/0 then we can all file bugs when apps don't "comply with the standard". | 15:41 |
| benji | heh, I just noticed that our board has a "Story 1" and a "Story A". I approve. | 15:42 |
| gary_poster | :-) | 15:42 |
| hatch | inside joke? | 15:43 |
| hatch | oooookk one more reviewer for https://codereview.appspot.com/7946043 going.....going..... | 15:50 |
| Makyo | Gone. | 15:50 |
| hatch | ^5 | 15:50 |
| Makyo | Need a micro-review or two (hatch did one, think it's an LGTM? Not sure) - https://codereview.appspot.com/7884044/ | 16:03 |
| hatch | yes sorry - I just didn't want to put LGTM if that was a typo | 16:04 |
| Makyo | No worries. Units just work a little differently. | 16:05 |
| gary_poster | bcsaller_, new Jenkins failures are worse: they never get past "subprocess.CalledProcessError: Command '['juju', 'status', '--environment', 'juju-gui-testing', '--format', 'json']' returned non-zero exit " | 16:22 |
| bcsaller_ | gary_poster: yeah, its no longer even building an environment from shell on the machine, I've been looking into it | 16:22 |
| gary_poster | ok thanks | 16:22 |
| gary_poster | hatch give the man (==Makyo, on https://codereview.appspot.com/7884044/) a LGTM :-) | 16:26 |
| gary_poster | (or not, if there's an issue) | 16:27 |
| hatch | oops :) done | 16:27 |
| hatch | hmm I don't think this current UI is going to work on a 7" tablet | 16:28 |
| gary_poster | hatch, that goes in the "design issues" category then :-) | 16:30 |
| rogpeppe | frankban: you have a revieew | 16:30 |
| rogpeppe | review, even! | 16:30 |
| hatch | :) ok so what I'll do is create a google doc with the issues for chrome and ff on this tablet | 16:30 |
| hatch | then we can generate tasks from that | 16:31 |
| gary_poster | cool hatch thanks | 16:31 |
| frankban | rogpeppe: thanks, nice comment suggestion! | 16:31 |
| rogpeppe | frankban: np | 16:31 |
| rogpeppe | gary_poster: i've got the CL in review that adds annotations to the StateWatcher. you might want to have a look, as it defines the format you'll see on the wire. https://codereview.appspot.com/7948043 | 17:05 |
| gary_poster | ack, thanks rogpeppe. benji ^^^ | 17:05 |
| gary_poster | rogpeppe, "arble":"" == deletion, I assume? | 17:07 |
| rogpeppe | gary_poster: hmm, good point. that shouldn't be possible in practice - a bad test case. | 17:07 |
| gary_poster | rogpeppe, cool. So each annotation change always sends us all values, AIUI, right? So we should replace any previous values, and so a deletion semantic is unnecessary. | 17:09 |
| rogpeppe | gary_poster: yes | 17:09 |
| gary_poster | cool | 17:09 |
| gary_poster | LGTM rogpeppe thanks | 17:09 |
| rogpeppe | gary_poster: thanks! | 17:09 |
| rick_h_ | hatch: I thought that only the defined ATTRS would be set from an initializer(cfg) but it looks like anything in cfg is getting an ATTR created for it? | 17:23 |
| hatch | yep | 17:27 |
| hatch | however - I consider it bad practice to not define the ATTR - else the code is referencing undocumented attributes | 17:28 |
| rick_h_ | hatch: ok, yea I just got through chasing down wtf views were getting their this.get('db') from and threw me for a loop | 17:29 |
| hatch | sorry, bad practice to not define an ATTR that you're using | 17:29 |
| hatch | yep I just dealt with the same thing....."where the heck is his attribute coming from" hehe | 17:29 |
| rick_h_ | all good, now that I realize they still work not being defined it'll be easier. | 17:29 |
| rick_h_ | stop looking in this file, find the calling piont | 17:29 |
| hatch | I have always had an issue with that because of the performance implications | 17:30 |
| hatch | but whatever | 17:30 |
| hatch | I belive as long asyou don't define a value it'll be lazily created | 17:30 |
| hatch | I can't remember the exact sequence though | 17:30 |
| rick_h_ | yea, but I guess the assumption is normally you pass fewer iterations through cfg than exist on an object in ATTRS | 17:30 |
| hatch | yeah - I have always had a small issue with it because all values passed to the initializer end up as attributes | 17:35 |
| hatch | imho the initializer should split them off into attributes and properties | 17:35 |
| rick_h_ | ok, after I figured out self.db the notification thing is sweet. Nice and easy to way to get a popup like that. :) | 17:51 |
| hatch | :) | 17:51 |
| gary_poster | bcsaller_, frankban ^^^ :-) | 17:51 |
| bcsaller_ | :) | 17:52 |
| frankban | cool | 17:53 |
| hatch | 72km winds right now....I'd probably kill myself if I went kiting haha | 17:54 |
| rick_h_ | great, the rest of us have "what docs do you have in case you're hit by a bus" and for hatch it's "what docs have you left in case you fly a kite?" | 17:54 |
| hatch | haha | 17:54 |
| hatch | my kite is only rated to 40kph by the manufacturer - so I'm not even sure it would be able to be controlled :D | 17:55 |
| rogpeppe | benji, gary_poster: here's a CL that adds a field to Service, by way of an introductory example of how to add more info to the AllWatcher: https://codereview.appspot.com/7950044/ | 18:11 |
| benji | cool | 18:12 |
| benji | (the whole fancy spacing thing is irritating in diffs, especially when you can't control the whitespace sensitivty of the diff-producer) | 18:13 |
| rogpeppe | benji: agreed. i would have been happy with no field alignment, but there y'go. | 18:15 |
| rogpeppe | benji: turning off ws sensitivity is problematic for other reasons, unless you have a diff that's knows about lang specific quoting rules. | 18:16 |
| rogpeppe | that brings me neatly to eod; g'night all | 18:17 |
| benji | 'night rogpeppe | 18:17 |
| gary_poster | night | 18:25 |
| === deryck is now known as deryck[lunch] | ||
| hatch | Makyo: are you around? | 18:38 |
| hatch | to talk about mobile | 18:38 |
| Makyo | hatch, making lunch now. Will be around to type in a few, hangout after. Unless you want to watch me eat noodles or something. | 18:39 |
| hatch | haha it's ok I pmd you some lunch reading | 18:39 |
| hatch | :) | 18:39 |
| hatch | lemme know when you're done lunch, no rush | 18:40 |
| Makyo | hatch, cool, thanks. | 18:40 |
| Makyo | hatch, ping | 19:06 |
| hatch | yo | 19:06 |
| hatch | guichat? | 19:06 |
| Makyo | hatch, yep, on my way. | 19:06 |
| === deryck[lunch] is now known as deryck | ||
| hatch | fyi all - it looks like node and npm have been updated to fix the error we were seeing | 20:09 |
| bcsaller_ | ahh, good | 20:10 |
| hatch | so someone can update that...or I can later | 20:13 |
| hatch | :) | 20:13 |
| hatch | any idea which mailing list I should ask about which browser ubuntu phone uses? | 20:21 |
| hatch | ubuntu tech? | 20:21 |
| hatch | Online I am seeing showshoe and webkit | 20:21 |
| hatch | although snowshoe IS webkit I think | 20:21 |
| rick_h_ | hatch: good ? I was wondering about the browser on there. I've ping'd aquarius about it but he's EU so might not hear back until tomorrow. | 20:25 |
| hatch | ahh ok - I can wait | 20:26 |
| hatch | I am trying to figure out what sets the height of the environment | 20:26 |
| hatch | bcsaller_: are we defining the height of the environment somewhere? | 20:27 |
| bcsaller_ | hatch: the viewport module handles that stuff | 20:28 |
| hatch | ah hah | 20:28 |
| hatch | of course the only one I haden't looked in yet | 20:28 |
| hatch | :D | 20:28 |
| rick_h_ | hatch: so told you want to chat with osomon about the browser. | 20:30 |
| rick_h_ | and let me know what you find out as I'm very curious :) | 20:30 |
| hatch | sure thing - doesn't look like he is online right now | 20:31 |
| rick_h_ | hatch: yep, spain so have to catch tomorrow | 20:31 |
| hatch | good news is that I have it so you can actually use most of the gui in FF already | 20:31 |
| hatch | in horizontal view | 20:32 |
| hatch | we aren't setup for vertical view | 20:32 |
| rick_h_ | yea, wouldn't expect it | 20:32 |
| rick_h_ | hatch: jcsackett https://codereview.appspot.com/7947045 for your consideration if you guys get time. Know it's late so maybe tomorrow. | 20:44 |
| hatch | yep I can do it now | 20:44 |
| hatch | it's only 2:$4 here | 20:44 |
| hatch | 2:44 | 20:44 |
| rick_h_ | hatch: oh, well 2hrs ahead here so I'll be heading out soon :) | 20:44 |
| hatch | does this need qa or just code review? | 20:44 |
| rick_h_ | hatch: I think just review. It doesn't effect anyone else. | 20:45 |
| hatch | alrighty | 20:45 |
| rick_h_ | hatch: if you QA it I need to check the status of the charmworld staging to make sure it's got the update for the ajax requests to pass | 20:45 |
| hatch | it's alright | 20:45 |
| hatch | :) | 20:45 |
| rick_h_ | which I will do tomorrow though. Next branch I'll wire up the button to swap between sidebar/full screen view and get that landed so it can be loaded with a button that does something :) | 20:46 |
| hatch | kewl | 20:46 |
| hatch | do you know when we should expect the yui responsive grid change? | 20:47 |
| rick_h_ | hatch: no...honestly had hoped to have it yesterday, then today, now hoping tomorrow. | 20:47 |
| hatch | alright no problem - was just making css changes for this mobile stuff so I'm not sure if they will still be there come the changeover :) | 20:48 |
| rick_h_ | yea, I'm going to guess it'll be a mess to merge. I imagine his branch is going to be lots of little edits all over | 20:48 |
| rick_h_ | hatch: https://code.launchpad.net/~huwshimi/juju-gui/replace-with-yui-grids looks like his branch pushed up yesterday | 20:49 |
| rick_h_ | for an idea | 20:49 |
| hatch | oh cool I'll check that out | 20:49 |
| jcsackett | rick_h_: i'm looking now as well. | 20:49 |
| rick_h_ | yea, always good to know you can go do http://code.lp.net/~username and see what branches they've got ;) | 20:49 |
| rick_h_ | thanks jcsackett | 20:50 |
| rick_h_ | heh, actually the branch is smaller than I would have thought hatch | 20:51 |
| rick_h_ | well I'm out. Have fun guys. | 20:51 |
| hatch | cya | 20:52 |
| hatch | I think I'm going to write a plugin for the linter which doesn't allow my_var :P | 20:54 |
| rick_h_ | hatch: now careful, I happened to look at things in config-debug/prod and I see a bunch of my_var :P | 20:57 |
| rick_h_ | so look around before you knock it, I might be keeping with 'convention' for the thing at hand | 20:57 |
| hatch | I'm pretty sure the convention was changed to be camelCase | 20:57 |
| hatch | to match javascript in the rest of the world :P | 20:57 |
| hatch | so there is a lot of legacy my_var | 20:58 |
| rick_h_ | ok, just saying I did go through and look through my code to try to make it camelHappy... :P | 20:58 |
| hatch | I think you missed a few files when you went through it :) | 20:58 |
| rick_h_ | hatch: ok, that's possible then as well ;) | 20:59 |
| hatch | haha - don't worry I'm not going to write 'camelCase' at every instance | 20:59 |
| hatch | just in the notes :P | 20:59 |
| hatch | did you use writeOnce because it could be set multiple times by accident? | 21:00 |
| hatch | or just as a precaution? | 21:00 |
| rick_h_ | hatch: no, honestly I don't think it's true of 99% of the attrs | 21:00 |
| rick_h_ | but it's on every single other attr so I added it along with it | 21:01 |
| hatch | ok I'm pretty sure there is a performance hit setting them to writeOnce | 21:01 |
| rick_h_ | yea, I'm on board with wiping that off the face of the models tbh | 21:01 |
| rick_h_ | just noise | 21:01 |
| hatch | haha ok I'll make a note in the review | 21:01 |
| hatch | I just wanted to ask first :) | 21:01 |
| rick_h_ | no, it's because the models are created from api data but there's no 'save' back to the store | 21:02 |
| rick_h_ | so none of those attributes from the model should be 'writable' but I think we can just ack it and move along vs writeOnce on each attr | 21:02 |
| rick_h_ | doh, I said I was gone...really am this time lol | 21:02 |
| hatch | haha | 21:03 |
| hatch | cya | 21:03 |
| gary_poster | hatch and Makyo, or whomever is still around, https://codereview.appspot.com/7896045 is available for review. | 22:44 |
| Makyo | gary_poster, on it. | 22:44 |
| gary_poster | thank you very much Makyo | 22:44 |
| hatch | okee | 22:45 |
| gary_poster | thank you | 22:45 |
| hatch | there should be a warning at the end of that url | 22:46 |
| hatch | lol | 22:46 |
| hatch | gary_poster: have you ran these tests? | 22:57 |
| gary_poster | hatch, yes, locally and in ie10 | 22:57 |
| hatch | and a follow-up does it have the new:gui: namespace stuff in this branch | 22:58 |
| hatch | ? | 22:58 |
| hatch | The init of the juju app now looks for an element in the index.html file which isn't there right now | 22:59 |
| gary_poster | hatch, yes, the diff is from when I branched, which was from before your branch. Did I add a test that will conflict with your changes? I could merge trunk preemptively and check | 22:59 |
| hatch | so this test should fail | 22:59 |
| gary_poster | ah ok. I'll merge trunk and look, thanks | 22:59 |
| gary_poster | that will mess up the future diff but hopefully that is ok | 22:59 |
| hatch | well | 23:00 |
| hatch | I could give you the line to fix it | 23:00 |
| gary_poster | sure, that would help! thanks. :-) I'll still need to manually merge though | 23:00 |
| hatch | http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/view/head:/test/test_notifications.js#L76 | 23:01 |
| hatch | yeah | 23:01 |
| hatch | I don't really like this - this is going to happen every time we instantiate app now | 23:02 |
| hatch | I'll create a ticket for me to find a better place to attach that listener in app.js which is causing the issue | 23:02 |
| gary_poster | hatch, tests passed | 23:02 |
| gary_poster | trying again with lots of refreshing... | 23:02 |
| hatch | really...interesting | 23:03 |
| hatch | http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/view/head:/app/app.js#L389 | 23:03 |
| hatch | that should cause it to fail without that element on the page | 23:03 |
| gary_poster | agreed. dunno. will put a break point in test | 23:04 |
| gary_poster | oh weird | 23:04 |
| gary_poster | test fails in isolation but passes together. Something is not isolated properly. :-/ | 23:05 |
| hatch | ahh darn | 23:05 |
| hatch | my guess is that element is hanging around - I must have screwed up and forgot to destroy it after one of the previous tests | 23:05 |
| gary_poster | welll... | 23:06 |
| gary_poster | ah, hatch, yes | 23:06 |
| gary_poster | and hatch, I could make it fail by changing line 389 to this.get('container').one('#nav-brand-env').on ... | 23:07 |
| gary_poster | oh... | 23:07 |
| gary_poster | but that is not supposed to be in the container is it | 23:07 |
| gary_poster | back in a few | 23:07 |
| hatch | nope it's outside of the container | 23:08 |
| gary_poster | back | 23:23 |
| hatch | So the purpose behind that event listener is to make sure that we capture any clicks to that logo so that we can display the environment view properly | 23:25 |
| hatch | unfortunately that element is always on the page because it's in the index not in a template | 23:25 |
| hatch | so maybe we could move it into a rendered template? | 23:25 |
| hatch | and then it woudln't break our tests | 23:25 |
| hatch | brb in 15 | 23:33 |
| gary_poster | we could also simply conditionally test for that element. <shrug> | 23:37 |
| hatch | yeah we COULD do that | 23:53 |
| gary_poster | hatch I'm removing the nav-brand-env after each test to see what passes then... | 23:53 |
| gary_poster | ooh, bunches | 23:54 |
| gary_poster | hatch, how do you want to resolve this? | 23:55 |
| hatch | could you edit the app.js file to put a conditional around that event listener? | 23:55 |
| hatch | that probably makes the most sense else every time we create a new instance of app we'll have to make sure that it's in the dom | 23:56 |
| gary_poster | yeah, that's what I was gonig to try next. Then I would rip out the test addition, because you don't seem to look at | 23:56 |
| gary_poster | it in the test | 23:56 |
| hatch | yeah sorry about that | 23:56 |
| hatch | tonight I'm going to set up remote debugging for my tablet in hopes I can solve that click issue in the environment tomorrow | 23:57 |
| gary_poster | awesome. Then you can show me how to do that later :-) | 23:58 |
| gary_poster | that change makes all tests pass | 23:58 |
| hatch | haha can do! | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!