/srv/irclogs.ubuntu.com/2011/11/23/#launchpad-dev.txt

=== StevenK changed the topic of #launchpad-dev to: qastaging is down, but being worked on. |https://dev.launchpad.net/ | On call reviewer: StevenK | Critical bugtasks: 292
wgrantErm00:38
wgrantr14367 is a little odd00:38
StevenKOh?00:40
wgrantNaming cookies after the LP username.00:40
wgrantVery odd.00:40
StevenKqas looks okay to me, is the topic stale?00:41
wgrantIt's been back for an hour or so.00:42
=== StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: StevenK | Critical bugtasks: 292
StevenKGood enough for me.00:42
wgrantlifeless: Was there consensus on which cookie approach to take?00:45
lifelesswgrant: see the discussion on the MP that just landed00:50
wgrantHmm, OK.00:51
wgrantStill seems pretty hideous to include the (mutable) username in the cookie.00:51
lifelessagreed00:52
lifelessderyck is coming around00:53
pooliewallyworld__, i guess feedback about the specifics of the disclosure ui would be beside the point?01:28
wgrantProbably, but still useful.01:29
wallyworld__poolie: for the email to the dev list yes. but huwshimi and mrevell etc would love to hear feedback01:29
wallyworld__poolie: since we are iterating based on screenshots from 18 months ago and user testing now etc01:30
wallyworld__poolie: also, that ui you saw from the email is now old. we're working on a new version, something like http://people.canonical.com/~huwshimi/policy7.png01:33
poolieok01:34
pooliethat looks a lot better01:34
* poolie cancels his draft01:35
pooliemaybe you can follow up with that?01:35
wgrantfuuuuuuuuuuuuu01:43
wgrantbuildbot failed again.01:43
* wgrant disables yuixhr01:43
wgrantgary_poster: Around?01:43
gary_posterwgrant: I'm working on it01:44
wgrantactual = ["Failure in lp/app/javascript/tests/test_lp_client_integration.Cache data.test_logged_in_user_has_cache_data: Unexpected error: Can't find variable: url",01:44
StevenKwgrant: I thought you did that last night?01:44
wgrantStevenK: I decided to give it another run to sort itself out.01:44
wgrantIt didn't.01:44
StevenKwgrant: Or was that just a threat?01:44
wgrantgary_poster: Any reason not to disable it now?01:44
wgrantIt's blocked all builds for more than a day now.01:44
wgrantAh, I see your email.01:45
wgrantThanks.01:45
wallyworld__poolie: the ui from the screenshot is not quite ready yet. the point of the email wasn't get get feedback on the disclosure ui, but the technology used to produce the mockups01:48
poolieyeah i understand01:49
poolieit was just an example01:49
pooliei think it's great that you did an interactive mockup01:49
wallyworld__it took a bit of work (lots of javascript hacking) but it's got some great feedback into the development of the feature01:51
wallyworld__if we can speed up the process by which we can deliver such mockups and end up with less wastage (throwaway code), i think that will be great01:51
pooliei wonder if there is any path to having it running in an actual lp instance but with no backend02:07
pooliei don't know if that would actually be harder02:07
wallyworld__poolie: pyramid or grok (ie a lightweight, easily deployable app server with zope support) i think may be easier02:11
lifelesswallyworld__: is there any reason you can't just edit templates in-place on lp ?02:19
lifelesswallyworld__: with the same persistence throwaway story ?02:19
wallyworld__lifeless: wouldn't that involve landing stuff via pqm or whatever?02:19
wallyworld__or do you mean ssh into the deployment server02:20
wallyworld__and edit directly via the file system?02:20
rick_h_wallyworld__: is this up some place to play with? (seeing screenshot)02:20
lifelesswallyworld__: ec2 demo02:21
rick_h_I'm curious what the edit link does, it seems like it'd just be on the person to overlay a ui to add/edit permsisions for that user vs per permission02:21
wallyworld__rick_h_: the latest screenshot isn't (yet). only the earlier version as per the email02:21
lifelesswallyworld__: e.g. just edit in your own local copy to hearts content, then shove it up on the ec2 instance (or canonistack or ...)02:22
wallyworld__lifeless: ah, that may work. i haven't used ec2 demo before02:22
wallyworld__rick_h_: the edit icon per person uses a ChoiceSource popup to edit the permissions for that person02:23
wgrantAs discussed on the call yesterday, ec2 demo probably doesn't quite work any more, but would be easy enough to fix.02:23
rick_h_wallyworld__: got it, missed the link when I read your email originally02:24
wallyworld__the thing to remember also is that it has to be a RAD environment. hacking javascript and static html fits the bill there. but we want a bit more, like templates etc02:25
rick_h_wallyworld__: are you guys using a JS tpl language at all?02:25
rick_h_YUI is building in support for handlebar in their MVC stuff, but I've used jquery-tmpl and underscores stuff02:25
wallyworld__rick_h_: no. but we would look to use mustache i guess since that's now in our technology stack02:25
rick_h_yea, I think deryck and the guys are using it on their feature landing02:26
wallyworld__(it wasn't when we started the mockup)02:26
rick_h_sorry, handlebar/mustache02:26
rick_h_I group the two together when I think of them02:26
wallyworld__sure :-)02:26
rick_h_I've always thought the couchapp stuff would be awesome for mockups, but I think it's a bit far from where you want to be02:27
rick_h_wallyworld__: small ui thing, curious why the "Add an Observer" wouldn't fit over on the right like a "Report a bug" or "Register a branch" type item?02:30
* rick_h_ is checking out when different ui mechanisms are used like that02:31
wallyworld__rick_h_: it may well go there. we were working off the supplied screenshots that were done a while ago02:32
rick_h_ah ok02:32
wallyworld__poolie: rick_h_: latest version http://people.canonical.com/~ianb/disclosure/02:32
rick_h_sorry, it's interesting stuff so I like to pester :)02:32
rick_h_I'd definitely try to go lighter on the grey for more contrast02:33
rick_h_but cool02:33
wallyworld__rick_h_: please continue to ask questions. fwiw, a lot of what is there in the screenshots we are working of we agree needs to be fixed02:33
wallyworld__so were are using them as a starting point02:33
rick_h_what would be sweet, would be if the grey boxes were buttons, and the edit link only showed on hover02:34
rick_h_that would clean it up a bit readable wise imo02:34
wgrantThat was what I was sorting of thinking, yeah.02:34
wgrantThe edit links all over LP are really cluttersome.02:34
rick_h_I know that hover stuff gets into the way of mobile, but I'm not sure how mobile friendly we are atm anyway02:34
rick_h_wgrant: yea, I want to align them something fierce lol02:34
rick_h_I've wished I had started back when the new buglist stuff had first started lol02:35
wgrantHeh02:35
wgrantYeah02:35
rick_h_I like my lines through my ui/design I guess, though I hate css grids...so go figure02:35
wallyworld__rick_h_: lighter grey now done :-)02:36
rick_h_ah, much nicer02:36
rick_h_so why can I click on some and not others?02:37
* wgrant tries to dig up the faded edit icon.02:37
wallyworld__rick_h_: atm, if you are in a team role, you get full observer rights (thats that the original screenshots called for). i'd have to check to see if that's still the case02:37
wallyworld__s/team role/project role02:38
wgrantwallyworld__: It's no longer the case.02:38
rick_h_ah, default perms then02:38
wgrantIt's merely the default during project setup.02:38
rick_h_gotcha02:38
wallyworld__wgrant: np. i'll add the edit icons to those ones then02:38
wgrantAha, https://launchpad.net/@@/edit-grey02:43
wgrantThat's what I was looking for.02:43
wgrantSlightly less abhorrent.02:43
wgrantThe initial inline bug status/importance used that until hover.02:43
wallyworld__wgrant: i think anything is less abhorent than the yellow :-)02:45
wgrantSurely not.02:46
wgrant(I think ideally it would appear like a dropdown listbox, so it would have a down arrow instead of an edit icon. but lazr-js doesn't really do that sort of thing)02:48
wgrantwallyworld__: I don't think it's useful to show "View nothing" rather than just hiding that policy. But then how do we add a new one.02:49
wallyworld__wgrant: that's the dilemma02:50
wallyworld__wgrant: we could have a proper popup form/widget02:51
wallyworld__and a single edit icon in the table row02:51
pooliewallyworld__, are the pencils on the permissions really helping anything?02:51
wgrantOur JS UI toolkit is sort of limited :/02:51
wgrantYes, they're helping to lots of LP pages look crap :)02:51
pooliealso, having the bug/branch column have no text is a bit odd02:52
poolieand the whole layout looks a bit oddly cramped02:52
wallyworld__poolie: they are currently needed to allow the permissions to be edited02:52
poolietechnically needed?02:52
pooliehm02:52
wallyworld__poolie: the bug/branch column doesn't need text - it's the mechanism to navigate to the user's detailed disclosure page02:53
wgrantwallyworld__: Well, might be useful to say "1234 bugs, 2 branches" or something like that.02:53
poolieexactly02:53
poolieand make that a link02:53
poolieit just looks broken at the moment02:53
wallyworld__can do that02:53
pooliemm02:54
poolieto make the top of the thing less cluttered02:54
wgrantAlso, those are really an attribute of the "restricted observer" grant for a particular policy.02:54
pooliemaybe move "add an observer" to the distinguished actions portlet in the top right?02:54
poolielike for 'report a bug'02:54
poolie"The list of observers can be large. Use Search to look for particular users.02:54
poolieThe search term will be matched against name, Launchpad id, email.02:54
poolie"02:54
poolieseems like it doesn't really need to be there02:55
wallyworld__can do that too. the current layout is as per the supplied screenshots that were were given to start from02:55
poolieit's pretty obvious what search will be for02:55
wgrantwallyworld__: Are these mockups in a branch somewhere?02:55
pooliethe heavy black rule through the middle of the filters is a bit odd too02:55
pooliemight be better if all the filters were grouped within a box02:56
poolie"Show users which can view the project because they belong to a team which has permission."02:56
wallyworld__wgrant: lp:~launchpad-purple-squad/+junk/disclosure_mockups02:56
poolieto start with surely you mean "who" not "which"02:56
pooliebut02:56
pooliei don't know if that sentence is adding much02:56
poolieor what it's supposed to mean02:56
wallyworld__poolie: the black line is to separate the controls since the ones above the line cause a xerver call, the ones below filter in place02:57
poolie:/02:57
poolieok02:57
wallyworld__but rememeber, this is all exploratory, a starting point02:57
pooliesure02:58
pooliei'm exploring it :)02:58
pooliei think, if the line was not there02:58
poolieit would probably be clear that some of them act immediately and others need to be submitted02:58
wallyworld__huwshimi: you may want to cut n paste the above discussion for feedback into the devel process02:58
wallyworld__poolie: the line was added because users thought it wasn't clear02:59
huwshimiwallyworld__: I'll have a look02:59
pooliewhat specifically wasn't clear?03:00
wallyworld__huwshimi: thanks. there's been some good feedback to add to whatever the user testers come up with03:00
pooliemm03:00
huwshimiwallyworld__: OK great03:00
pooliei think it's a bit like the green link thing03:00
poolieyes it means something03:00
pooliebut is it something users actually need to know about?03:00
wallyworld__poolie: it wasn't clear that the bottom controls acted in place vs doing a submit03:01
poolieif they need to know about it to use the thing effectively, it needs to be a distinction they'll actually understand03:01
pooliehm ok03:01
wallyworld__poolie: i sort of agree with you. i was just following orders :-)03:01
poolieso they clicked it and03:01
poolie...03:01
poolietrying to understand why that would be confusing03:01
wallyworld__by that i mean i think it was indicated we need to distinguish between the two types of controls, but i could have mis remembered that03:02
pooliei don't think that's a goal in itself03:03
pooliethe point is just that people can understand how to use it03:03
wallyworld__so, so far we've gone from the original old screenshots to huwshimi's latest design which came from one round of user testing plus the ever evolving feature requirements03:03
poolienobody else specifically distinguishes them so i'm really skeptical that we need to03:03
wallyworld__the end users haven't yet seen this 2nd iteration03:03
pooliei think if we ask them "will foo cause a round trip to the server" they may not know but that's not a practical question03:04
wallyworld__poolie: it's not so much a round trip to server thing vs a responsiveness thing. in-place obviously much quicker03:05
wallyworld__but perhaps the distinction isn't required03:05
pooliemm03:05
pooliepeople will find out it's slow easily enough :)03:05
wallyworld__like with the rest of lp untilthe recent performance improvements :-)03:06
wallyworld__so, the idea now is to iterate to get to the point where the ui reflects the latest functional requirements. then we can bikeshed^H^H^H^H^H file tune :-)03:07
StevenKwgrant: Can you have another look at https://code.launchpad.net/~stevenk/launchpad/use-userHasPriviledges/+merge/82967 ?03:16
wgrantStevenK: Is DSP.driver necessary?03:18
wgrantAlso, I don't see any tests for SP/DSP.drivers03:18
wgrantApart from that it looks disturbingly sensible.03:18
StevenKDisturbingly? :-/03:19
StevenKI'm not sure about DSP.driver, I don't think so, since the codepath checks if it implements IHasDrivers and then grabs .drivers03:19
wgrantRight.03:19
wgrantBut you seem to have added DSP.driver.03:19
StevenKAnd .drivers. I am a muppet.03:20
StevenKwgrant: Right, binned. Let me write a test for both.03:21
wgrantThanks.03:21
wgrantShould be pretty quick :)03:21
huwshimiwallyworld__: Which discussion exactly did you want me to look at?03:24
wallyworld__huwshimi: just the feedback about the mockup which people saw after i sent the email to dev. eg search form, edit icons etc03:24
StevenKwgrant: Diff updated03:32
wgrantk03:33
wgrantStevenK: Are you sure those new tests aren't just [] == []?03:36
StevenKwgrant: Yes. I've added self.assertIsNot([], obj.drivers) where obj is the distro or the series and the tests still pass.03:39
wgrantStevenK: k03:40
wgrantHah03:40
StevenKwgrant: Do you want me to commit and push that change, or you don't care?03:40
wgrantfhjwejiowefjio03:40
wgrantStevenK: You know xfce4-smartbookmark-plugin?03:40
wgrantHow gina's been complaining about it for weeks?03:41
wgrantStevenK: s/IsNot/NotEqual/, but yes.03:41
StevenKWas that the non-free->main madness that the ftp-masters screwed up?03:41
wgrantStevenK: No, it fails to unpack due to a patch referenced in series that doesn't exist.03:42
wgrantIt turns out that dpkg-dev is actually hideous.03:42
wgrantAnd having reliably unpackable packages would be too easy.03:42
wgrantThere are vendor-specific series files.03:42
StevenKOh, right, that one.03:42
wgrantSo this unpacks fine on Debian, but not Ubuntu.03:42
StevenKBwaha03:42
wgrantBecause the bogus patch is mentioned in debian/ubuntu.series.03:42
StevenKSigh03:42
wgrantWho could possibly think it was a good idea.03:42
wgrantTo make it ununpackable in that case.03:43
StevenKSo we need to patch dpkg to complain but not abort?03:43
wgrantNo, we probably need to slap whoever designed 3.0 (quilt) like that.03:44
wgrantI'm not sure what dpkg-source can sensibly do here :/03:44
=== thumper is now known as thumper-afk
wgrantI wonder if --no-preparation works.03:45
wgrantIt does not.03:45
wgrantAh, --skip-patches03:45
StevenKWe do not care about patches for gina, surely03:45
wgrantExactly.03:45
StevenKwgrant: Not so fast though -- does Lucid's dpkg support --skip-patches ?03:46
wgrantI'm about to find out.03:46
wgrantIt does.03:46
StevenKwgrant: Perhaps we should get wallyworld__ to fix it. Then he can touch Soyuz and wish he didn't.03:46
wgrantI wonder what happens if I transfer the unpacked tree from an Ubuntu machine to a Debian one.03:47
wgrantStevenK: Do you feel like filing a bug in Debian?03:47
* wallyworld__ ducks for cover03:47
wgrantSince I hate the BTS.03:47
StevenKAgainst dpkg?03:47
StevenK:-)03:47
wgrantHah03:49
wgrantAs much as I would like to consider the vendor-specific unpacking rules to be a grave bug, I don't think they'd like me much for that.03:49
StevenKwgrant: Why don't you just configure reportbug?03:50
wgrantSTAB03:50
wallyworld__poolie:  wgrant: i added the grey edit icons03:50
wgrantwallyworld__: I doubt they look good, but they're probably less offensive.03:50
* wgrant checks.03:50
wgrantHah03:51
wgrantThe circle is just slightly visible.03:51
wallyworld__less offensive is also what i'd say03:51
StevenKwgrant: Those two extra asserts are in the diff.03:51
wgrantBut still looks much better.03:51
wallyworld__yeah03:51
wgrantWith slight tweaking of the (otherwise unused, AFAIK) icon, it could look quite reasonable.03:51
wgrantIn fact, possible just drop the circle entirely.03:51
wgrantStevenK: lol03:52
wgrantStevenK: dpkg-source: warning: --skip-patches is not a valid option for Dpkg::Source::Package::V3::native03:52
wgrantStevenK: So it warns if you ask for reliable extraction all the time.03:52
StevenKFucking hell.03:52
wgrantIt still works.03:53
wgrantBut spews warnings.03:53
StevenKwgrant: However, here it unpacks, just exits 203:53
wgrantStevenK: Ah, is that so...03:53
wgrantI didn't think about that.03:53
wgrantI wonder what 2 means.03:53
StevenKI wonder if I can claim serious03:54
StevenKimportant, at the very least03:54
* wgrant watches for the wontfix03:55
* wgrant wonders why we don't have any running water.03:56
* wgrant goes hunting.03:56
StevenKwgrant: debian/patches/series is a symlink to ubuntu.series03:58
wgrantStevenK: In Ubuntu's version, or Debian's?04:01
StevenKDebians04:01
StevenKSo Debian's xfce4-smartbookmark-plugin has a patch applied to direct people to launchpad to file bugs04:02
StevenKRather than the BTS04:02
wgrantWhich version?04:03
wgrantIn 0.4.4-1, debian/patches/series doesn't exist.04:04
wgrantBecause Debian's patches were included upstream.04:04
StevenKI have 0.4.4-1 unpacked, and it's a symlink04:04
StevenKOh, that's dpkg-source being stupid04:05
wgrantHah04:05
StevenKNastygram filed04:06
StevenKwgrant: Now that I've wrangled the BTS for you, can you wrangle my MP for me?04:07
wgrantSorry, got distracted by lack of water.04:08
wgrantAnd dpkg-dev04:08
StevenKwgrant: Thanks, tossing at ec2.04:16
pooliewallyworld__, grey just looks kind of disabled04:23
wallyworld__yeah, i guess so04:23
poolieis there a technical thing that will break if there's no icon?04:24
wallyworld__but final styling will come later04:24
poolieor is it more just that we think people won't realize they can click it...?04:24
poolieyou get a hover underline04:24
pooliethat makes it pretty obviously clickable to me04:24
wallyworld__i think people expect an icon04:24
pooliehuwshimi, remove bug tags woo04:24
wallyworld__but i'll defer to the usability testing for what we need to do04:25
poolieyeah, me too04:25
pooliei just want the possibility of no pencils to be considered04:25
* StevenK stabs jtv04:25
pooliethe argument is basically just "we've always done that"04:25
jtvWhat have I done now?04:25
jtvOr are you just exploring different uses of pencils?04:26
StevenKjtv: Inflicted that damnable buildd-manager branch on me04:26
pooliehaha04:26
jtvSCORE!04:26
jtvI was going to mention that.04:26
jtvAt some point.04:26
jtvAlmost certainly.04:26
jtvCross my heart and hope to be just outside my home when a slight drizzle starts.04:27
pooliewallyworld__, so while we're waiting for the testing04:27
pooliei don't know, is there any other reason than just people might expect it?04:27
StevenKjtv: We've had 23mm here since 9am04:28
poolie.. i don't want to be a pain04:28
pooliei just really think it looks bad without adding anything04:28
pooliewe have to stop some day04:28
wallyworld__poolie: well, i  would think we need to be consistent with the test of lp04:28
wallyworld__s/test/rest04:28
StevenKwgrant: Debian #649680, since the BTS sucks.04:28
_mup_Bug #649680: Link titles of pages that aren't UTF8 appers as gibbrish <Hoborg IRC Bot:Fix Committed> < https://launchpad.net/bugs/649680 >04:28
pooliecause the rest of lp has totally consistent ui?04:28
poolie:)04:28
wallyworld__mostly04:28
StevenKpoolie: I thought you knew!04:29
wallyworld__more or less04:29
wallyworld__perhaps04:29
wgrantI think we need some indication that it's editable.04:29
poolieconsistency has value but04:29
wgrantA pencil is probably not it.04:29
poolieif you want to change things04:29
wgrantIt's really a list both.04:29
wgrantList box.04:29
poolieyou've got to start somewhere04:29
wallyworld__sure04:29
poolieso one pattern people use is a little [v] icon when it pops up a control04:29
poolieespecially a list04:30
wgrantExactly.04:30
wgrantThat's exactly what this is.04:30
wgrantBut lazr-js doesn't really do that sort of thing.04:30
pooliebut really on many sites just hover underline/color/background seems to be enough04:30
wgrantIt prefers invoking a popup somewhere else on the page.04:30
poolieit invites people to click and then they can see what happens04:30
wallyworld__my thinking is that lp is being rebranded atm, so we should wait for that to establish the guidelines04:31
wallyworld__we have other things to deliver04:31
pooliefair enough04:31
wallyworld__poolie: but i'm sure huw is open for suggestions on his rebranding work :-)04:32
poolie:)04:34
jtvStevenK: thank you for your sage comment on my branch.04:52
jtvThe bit about the vodka was particularly insightful.04:53
jtvI think you're right.04:53
StevenKAbout which bit?04:53
jtvThe vodka.04:54
lifelessthe vodka, clearly04:54
jtvwgrant: do you drink vodka?  I'm asking because we need another expert opinion on https://code.launchpad.net/~jtv/launchpad/bug-717969/+merge/8302204:55
nigelbA branch about vodka?04:56
* nigelb looks04:56
jtvnigelb: nooooooo!  Don't!04:57
jtvPoor man.  Went in without his Absolut goggles.  He'll never be the same.04:58
nigelbHahaha. Nice.04:58
nigelbI stayed away from the code. So not scarred.04:58
nigelbThe only read your description and StevenK's rerview.04:58
nigelb*review04:58
* jtv tearfully greets nigelb back04:59
nigelb:)05:00
wgrantYay05:01
wgrantUnicodeDecodeError in bson.loads05:01
StevenKLIFELESS05:02
nigelbooh. Caps Lock.05:03
lifelessStevenK05:05
StevenKlifeless: I was just guessing the Unicode issue was from oops-tools or something05:06
pooliehm so merging this loggerhead export patch raises the question of upgrading loggerhead as a whole06:52
lifelesssame as other source deps - land a change to LP to upgrade it06:59
=== thumper-afk is now known as thumper
poolieok08:34
poolietarball download works here; it should be ok to get it in tomorrow08:34
danhgMorning09:11
=== almaisan-away is now known as al-maisan
=== matsubara-afk is now known as matsubara
=== al-maisan is now known as almaisan-away
=== matsubara_ is now known as matsubara
=== almaisan-away is now known as al-maisan
rick_h_morning11:55
allenapGood morning rick_h_12:18
rick_h_wallyworld__: wgrant is there a branch for the disclosure stuff I can see/find to checkout?13:58
=== matsubara is now known as matsubara-lunch
cr3sinzui: hola, I noticed your branch ~sinzui/html5-browser/trunk doesn't seem to exist anymore, which one should I be using instead?14:34
sinzui!14:34
sinzuiI will find it and put it back14:34
cr3sinzui: thanks man, it's been really nice for testing yui14:35
sinzuicr3, I am helping another user. It will be a few minutes14:38
cr3sinzui: no worries, take your time14:39
sinzuicr3, can you see https://code.launchpad.net/~sinzui/html5-browser/trunk ?14:48
sinzuiI just branches lp:html5-browser14:48
sinzuiI just branched lp:html5-browser14:50
cr3sinzui: seems to work, but it didn't work when building overnight: https://launchpadlibrarian.net/85743890/buildlog_ubuntu-precise-i386.launchpad-results_0.13-1%7Eppa0.12.04_FAILEDTOBUILD.txt.gz14:50
sinzuiah. cr3, I think gary was changing lp's buildout rules. That was added recently. I created debs, but buildbot does not have them14:53
cr3sinzui: the package seems to have built fine for 10.04 and 10.10 though, only later releases failed. anything I should do?14:54
abentleyrick_h_: I don't see assert_change being used anywhere.  Is it dead code?14:54
rick_h_looking14:54
rick_h_yes, changed to using the .wait() thanks14:55
sinzuicr3. I will copy the package to precise. I may want to do that for a few other too14:55
cr3sinzui: the build also failed for 11.04 and 11.10 though14:55
sinzuiI am not sure what to say, maybe gary_poster can help since he added the source deps14:56
gary_posterwhat did I do?14:56
gary_posterAll I remember doing was adding html5_browser as a buildout dependency to lp14:57
cr3gary_poster: can you have a look at this build failure, seems to be related to buildout: https://launchpadlibrarian.net/85743890/buildlog_ubuntu-precise-i386.launchpad-results_0.13-1%7Eppa0.12.04_FAILEDTOBUILD.txt.gz14:57
=== abentley changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: abentley | Critical bugtasks: 292
gary_posterbut I didn't touch the debs14:57
gary_postersinzui, cr3, this seems to be the heart of the issue: http://pastebin.ubuntu.com/747089/14:59
gary_posterI don't think I touched html5 browser's buildout either, though I could be wrong14:59
gary_posterbut I don't think that error has anything to do with buildout anyway15:00
sinzuigary_poster, who put html5 in buildout? I only make deb packages15:00
cr3gary_poster, sinzui: I'll try rebuilding in the ppa and then maybe try locally with sbuild15:01
cr3sinzui: I put html5 in my buildout15:01
sinzuiah, thank you for clarifying15:01
gary_postersinzui, I put html5 in LP buildout.  That has nothing to do with this error15:01
cr3gary_poster: so we both have html5 in our buildouts, do you have a tarball in your sourcedeps branch or do you branch html5 directly in buildout?15:03
gary_postercr3 tarball15:03
cr3gary_poster: maybe I should do the same...15:03
abentleyrick_h_: It's really helpful if you include my nick.  This can be a noisy channel, but using my nick makes your replies stand out.15:13
rick_h_abentley: yea, sorry.15:13
rick_h_I forget that not all irc has nick highlighting and such. I know some do notifications so try to avoid doing that too much15:14
rick_h_thanks for the heads up abentley15:14
abentleyrick_h_: my IRC has nick highlighting, but your nick is the same colour as sinzui's.15:15
rick_h_gotcha15:15
abentleyrick_h_: When you mention my nick, you get highlighted orange, and I get a bell.15:16
abentleyrick_h_: I try to avoid pausing in tests for two reasons: 1. such tests often fail intermittently, 2. such tests take longer than they otherwise would.15:18
abentleyrick_h_: what is the operation you're needing to wait for?15:19
rick_h_abentley: agreed. I can see if I lessen the amount of test text I use the redraw on the clone element will be fast enough to not require it15:19
rick_h_abentley: I believe it's the updating of the content from the target, to the clone, and getting that to resize before I can check it's new height value15:19
abentleyrick_h_: can you just change the code to be synchronous, i.e. to only return once the new value is set?15:20
rick_h_it's going off off a YUI triggered event15:20
rick_h_I'll see if I can find a way there and I'll trace the actual draw/etc timings to make sure where the pause is15:21
abentleyrick_h_: whatever is triggering the event will not return until all the event's listeners have been run.15:21
=== matsubara-lunch is now known as matsubara
deryckhmmm, wonder why we got some css changes on qastaging but not all.15:27
abentleyrick_h_: On line 358, you specify CHANGE_TIME as a third parameter to target.plug.  What does that mean?15:28
rick_h_abentley: nothing, not sure how that got into that one call15:29
rick_h_removed15:29
abentleyrick_h_: Cool15:30
abentleyrick_h_: could you change "ns" to "module" per https://dev.launchpad.net/JavaScriptReviewNotes#Javascript_Module_formatting ?15:32
rick_h_abentley: sure thing15:32
deryckabentley, rick_h_ -- can one of you guys check qastaging for me.  are the importance colors the same as they were?15:33
abentleyderyck: they look different.  "Medium" is light blue.15:34
deryckabentley, ah nice.  So some odd caching on my end then.  cool.15:35
abentleyrick_h_: Do you need MAX_HEIGHT and MIN_HEIGHT to be separate variables, or would it work to do e.g. "min_height: {value: 10}"?15:38
deryckman, getting those "columns" to line up with real data is going to be really hard to do.15:39
rick_h_abentley: looking15:40
rick_h_abentley: I'll need to find a different way to tell if a value has been passed in at object construction time.15:42
rick_h_abentley: I'll add a test to picking up the min height from the current element (I think that's broken right now) but that needs to know if you passed a min height or not15:43
abentleyrick_h_: I think you could move that code into a valueFn.15:43
=== al-maisan is now known as almaisan-away
abentleyrick_h_: But why do want to pick up the min height from the current element?15:45
abentleyrick_h_: ISTM that the current element could be really big, and you could be reducing its size a lot.15:45
rick_h_abentley: well if in the html of your textarea you say height=xxx, I want to respect that and not resize it down to the default15:45
abentleyrick_h_: aren't textareas usually configured in rows and columns rather than height and width?15:47
rick_h_you can, but I think most move that to css these days15:48
rick_h_so if the css says the height is 150, I'll use that as the new min if you didn't specify one at your .plug() call15:48
rick_h_abentley: ^15:48
abentleyrick_h_: Okay.15:49
rick_h_looking at the valueFn, if I have set a value, it seems this.get('min_height') would return value into my valueFn() call15:49
rick_h_I'll check to make sure, if not, then I can remove the constant and move to a valueFn15:49
abentleyrick_h_: If you supply a value, the valueFn will not be used at all.15:49
abentleyrick_h_: If the valueFn return undefined, then the value will be used.15:50
rick_h_"If both the value and valueFn properties are defined, the value returned by valueFn has precedence over the value property,"15:50
abentleyrick_h_: right.15:50
abentleySo, the precedence is supplied value > valueFN > value15:50
rick_h_ok, I see. So using this.get('min_height') in the valueFn is safe because I know that no value was supplied15:51
abentleyrick_h_: in the valueFn, I don't see why you'd get('min_height') at all.  It would either return undefined or do infinite recursion.15:53
rick_h_right, it would just check if there was a css style to use15:54
rick_h_if so return it else undefined and value kicks in15:54
abentleyrick_h_: right.15:54
rick_h_thanks, I like that a lot better15:55
abentleyrick_h_: someone writing YUI was thinking the same way you are.15:55
rick_h_yea, I've not used valueFn and such. New toys!15:55
abentleyrick_h_: the condition at 186 looks very complicated.  Are you worried about the case where this._prev_scroll_height < this.get('min_height') ?16:01
rick_h_abentley: no, if the prev_scroll_height < min_height it falls through to the else case and sets the height to the min16:02
rick_h_I could wrap those into bool returning function calls with better names to help make it clearer perhaps16:03
abentleyrick_h_: how would prev_scroll_height be less than min_height>16:03
abentleyrick_h_: how would prev_scroll_height be less than min_height?16:03
rick_h_abentley: _set_start_height should make sure that never happens16:04
rick_h_but you asked if I was worried about that case, and even if it *were* to happen the else clause should cover it16:05
abentleyrick_h_: If it's safe to assume that this._prev_scroll_height < this.get('min_height'), then can't we rewrite the conditional as "if (scroll_height > this.get('min_height'))" ?16:05
abentleyrick_h_: Or maybe do new_height = Math.max(this.get('min_height'), Math.min(scroll_height, this.get('max_height')) ?16:07
rick_h_abentley: yes, I think you're right. I set the default _prev due to a bug in testing and I think you're right that it greatly simplifies the conditions there now16:08
abentleyrick_h_: cool.16:09
abentleyrick_h_: You talk about preventing resize in webkit.  What happens in firefox?16:10
rick_h_abentley: it doesn't allow users to drag/resize. Since it's based off a css property, I'd hope that if it's added they'd support the same standard16:10
abentleyrick_h_: yes, it does, in recent Firefoxes on Ubuntu.16:11
rick_h_ah, you're right. And the css rule applies there16:11
rick_h_abentley: so I'll generalize that comment then vs just calling out webkit16:12
abentleyrick_h_: okay.16:12
abentleyrick_h_: this._t_area is just meant as a more meaningful alias for this.get('host')?16:13
rvbaabentley: Can you please have a look at this MP? https://code.launchpad.net/~rvb/launchpad/revert-14355/+merge/8318316:14
rick_h_abentley: yes, shorter to type, specific, and prevents calling .get() multiple times16:14
abentleyrvba: I can do it next.16:14
rvbaabentley: I'm reverting the "improvement" I've made to an expensive query because the performance is worse.16:14
abentleyrvba: doh!16:15
rvbaabentley: oh, allenap agreed to have a look at it right now…16:15
abentleyallenap: have at it.16:16
abentleyrick_h_: why does _setup_css take a parameter?16:16
rick_h_abentley: no good reason.16:17
abentleyrick_h_: okay, please change it to use this._t_area, then.16:18
rick_h_done16:18
rick_h_abentley: you use the timeline tool in the chrome dev tools much?16:19
abentleyrick_h_: no, I don't use chrome much.16:19
rick_h_ok16:19
abentleyrick_h_: what does it do?16:20
rick_h_it shows the low level browser calls/events, layout, paint, event fired, etc16:20
rick_h_I'm trying to use it to figure out why I need that pause16:20
rick_h_that's ok, nvm. Just curious16:20
jcsackettsinzui: would review for the mockup wallyworld and i whacked together be sending it over to dan and huw for comments, or ... ?16:21
sinzuijcsackett, I think I did16:22
jcsackettsinzui: ok. i'll throw the card into review then.16:22
jcsacketti'll also take a quick look at that padlock icon on loggerhead; i think it's a quick fix.16:22
sinzuifab16:22
abentleyrick_h_: Is it a case where "setStyle('foo', 'bar'); getStyle('foo')" does not return "bar"?16:22
rick_h_abentley: it seems to be. I can see the event fired, but it doesn't show it hitting the event catching code16:23
rick_h_and then I see it running a layout call because I called getStyle16:23
rick_h_which should cause it to pick up the recalc, but it's like the event callback isn't done yet16:23
rick_h_but like you say, it should be16:23
sinzuiI think the main reason I do not blog like it is 2005 is because the spammers comment like it is 201216:23
abentleyrick_h_: What event?  this.t_area.on('valueChange' ?16:24
rick_h_abentley: yes16:24
rick_h_that callback has to complete before I can grab the updated height for the test to verify the change16:25
abentleyrick_h_: so if you set a breakpoint in resize, it never stops there?16:25
rick_h_it does, but the timeline doesn't show it calling out to resize. What I'm trying to figure out is how the new value is getting pulled as the same as the original before the callback is complete16:26
abentleyrick_h_: so in FF, test_initial_resizable and test_max_height pass without the wait.16:31
abentleyrick_h_: the other two fail.16:31
rick_h_abentley: it fails for me16:31
rick_h_initial_resizable16:31
rick_h_in FF16:32
rick_h_I've commented out the rest and only working on trying to get the initial_resizable working without the wait16:32
rick_h_in both browsers, without luck so far16:32
abentleyrick_h_: weird, it consistently works for me.16:32
abentleyrick_h_: FF 816:33
rick_h_abentley: ah, I'm on 7.0.116:33
abentleyrick_h_: In initial_resizable, there's no event involved, is there?16:34
rick_h_abentley: true16:34
rick_h_it just runs resize on the first call after it determins min/max16:35
abentleyrick_h_: Oh, there are two waits in that one, and I missed the second.16:35
abentleyrick_h_: fails for me too, now.16:35
rick_h_abentley: ah ok16:35
abentleyrick_h_: I track the event down to value-eventchange.js:15716:38
abentleyrick_h_: and it appears to do polling.16:38
rick_h_abentley: gah, that makes a lot of sense then16:39
rick_h_abentley: makes a lot of sense. If I changed the CHANGE_TIME to 50ms it would always pass, but under that it would fail, even with the wait16:40
rick_h_and that's the POLL_INTERVAL16:40
abentleyrick_h_: In your tests, do you have a philosophical objection to firing the event yourself?  Or invoking the functions directly?16:40
rick_h_abentley: the simulate code doesn't support valueChange16:41
rick_h_abentley: so I had to mimic a keypress event which it does support16:41
deryckabentley, hi. I realize you're on call today, but we need someone from our squad to get a deployment done today.16:42
rick_h_abentley: the event code dupes the content to the clone so I could pull that out in order to make it callable for testing purposes16:42
deryckabentley, and I think it's a bit too soon to ask that of rick_h_ :)16:42
abentleyrick_h_: I think that would be best.16:43
rick_h_abentley: ok, I'll ajust for that and let you know when I get that into place16:43
abentleyrick_h_: Right now, it's a bit of an integration test, where YUI is part of the system under tests.16:43
rick_h_sounds like deryck has plans for you :)16:43
rick_h_abentley: agreed, thanks for pointint out the polling16:43
abentleyderyck: Okay, but it's not something I have much experience with, either, TBH.16:44
deryckabentley, it's not too hard.  See the qa report here:  http://lpqateam.canonical.com/qa-reports/deployment-stable.html16:45
deryckabentley, you just need to pester people to get qa done or do qa for anyone who is sleeping, if you can.16:45
deryckabentley, we need to deploy up to r14369.16:47
deryckabentley, and either I or flacoste can help a bit if you get stuck.16:47
deryckabentley, then once the qa is caught up, you update the LPS page and ask a losa to do the deploy.16:47
abentleyderyck: I don't think we can deploy, because r14355 is bad, and the rollback hasn't landed yet.16:50
deryckah crap.16:51
deryckflacoste, ^^16:51
abentleyderyck: that rolllback is pending, though.16:51
rvbafwiw I've just reverted r14355 (r14374).16:51
deryckso that's some better then.16:51
deryckstill probably 4-5 hours to the build completes then.16:52
deryckabentley, that may be ok, then.  By the time you get all the qa chased up, that rev could be merged.  right?16:52
deryckassuming buildnot plays along with us.16:52
rick_h_can either of you load code.qastaging without a timeout?16:53
abentleyrick_h_: did it recently.16:53
rick_h_I thought things were still 'not right' since I've not been able to load to look at my QA16:53
rick_h_ok, I'll keep trying then16:53
abentleyrvba: Cool.16:54
abentleyderyck: Sure, though that means more revisions to QA.16:54
flacostederyck, abentley: so the next deployable revision will be 14374, but there is still some QA to go still16:56
deryckabentley, yeah, sorry, just more to qa.  And you don't have to do it all either.  You can ask for help, for the original author to qa, etc.16:57
deryckabentley, the idea is just that you'll be the one to poke people and prod the process along.16:58
mterryflacoste, gary_poster, benji: barry suggested I ask you a question I have about zope.  I'm trying to fix a FTBFS for zope2.12 because the package is hardcoded to use python2.6.  It will build if I change that to 2.7, but do you have any ideas how likely that is to work run-time?  2.13 apparently explicitly added support for python2.7.  Do you know if that was a no-op or if there's a patch floating around I can use?16:59
abentleyrvba: what about 14366?17:00
rvbaabentley: I knew you would ask ;)… I need the revert to be on qastaging to qa that revision.17:00
gary_postermterry, I'm afraid I have no idea--I haven't had any connection to Zope 2 in a very long time.  benji is not around today, but probably is similar.  I can give you two emails of people I'd recommend asking in a privmsg.  Would that be helpful?17:01
mterrygary_poster, sure, thanks17:01
abentleyrvba: I don't understand.  It looks like it needs a rollback or a bugfix.  Are you doing either one?17:01
rvbaabentley: Well, I can qa it on anther page… but the page I'd like to see now issued a very bad query that makes the page time out.17:03
rvbas/issued/issues/17:04
abentleyrvba: it's marked qa-bad.17:04
abentleyrvba: it doesn't sound like it needs qa.17:05
rvbaabentley: both revisions target the same bug.17:05
rvbaabentley: r14355 is qa-bad (I've reverted it), the other one needs qa.17:05
rvbaabentley: https://bugs.launchpad.net/launchpad/+bug/86794117:06
_mup_Bug #867941: person:+activereviews times out <affects-ubuntu> <code-review> <qa-bad> <timeout> <Launchpad itself:Fix Committed by rvb> < https://launchpad.net/bugs/867941 >17:06
rvbaabentley: not sure how to deal with that situation…17:06
abentleyrvba: Ah.  CAN WE PLEASE HAVE REVISION BASED QA?  PRETTY-PLEASE?17:06
rvbaRight!17:06
deryckrick_h_, this matches the new spinner you added, right?  http://people.canonical.com/~deryck/spinner-big.gif17:07
abentleyrvba: So once your revert is landed, you'll be able to QA the other revision, which may or may not be bad.17:08
rvbaabentley: right. Please take into consideration the fact that I'm almost EODed, but I can come back later this evening to do that qa.17:09
* deryck see doing a deploy has driven abentley to shouting :)17:09
rvbahehe17:09
abentleyderyck: This is how I always feel when I have to interact with the deployment reports.  They don't tell me what I want to know, and they don't tell me the truth.17:11
rvbaabentley: waterfall says the ETA for the build is ~2h.  How long after that for the revisions to be deployed to qastaging?17:11
flacostervba: if you leave instructions on how to do the qa, i can take care of it17:12
abentleyrvba: I don't know how long that takes.17:13
abentleyrvba: I guess we could ask a losa to help the process along.17:14
rvbaflacoste: thanks for the offer, I'll write the instructions on the mp, but I'll nonetheless pop up in 2h30 min to see if I can do the qa myself.17:16
flacosteok, thx17:17
rick_h_deryck: yes, that matches17:22
rvbaflacoste: abentley I've just updated the mp https://code.launchpad.net/~rvb/launchpad/activereviews-bug-867941-eagerload2/+merge/82904. Right now the page it talks about on qastaging is not accessible because it times out (hence my previous revert of an attempt to improve one of the huge queries).17:23
flacostervba: perfect!17:25
deryckrick_h_, thanks!17:25
deryckmy xchat indicator seems to have died.17:26
=== salgado is now known as salgado-lunch
rvbaI'm off now but I'll be back later tonight to see if I can do my qa…17:27
=== salgado-lunch is now known as salgado
abentleyderyck: https://code.launchpad.net/~benji/launchpad/bug-877195-code/+merge/80619 does not have enough information for me to do QA, and benji isn't around.18:15
abentleyrick_h_: Have you been able to do QA on lp:~rharding/launchpad/bug_814697_missing_spinner ?18:17
rick_h_abentley: no, I can't get code.xxx to load other than the root page18:18
rick_h_I've been trying the last hour18:18
abentleyrick_h_: https://code.qastaging.launchpad.net/bzrtools/+activereviews works for me.18:20
abentleyrick_h_: also https://code.qastaging.launchpad.net/bzrtools/+merges18:22
rick_h_abentley: thanks, looking for a good sample case now18:23
rick_h_gah, this is nuts18:37
rick_h_abentley: so do the branches actually exist on qastaging if I wanted to "fake" enough to get something to QA?18:37
rick_h_I've tried three different ones and just get "error not a branch" from bzr18:37
abentleyrick_h_: yes, pushing to qastaging should work.18:38
rick_h_ok18:38
abentleyrick_h_: bzr push lp://qastaging/~abentley/bzrtools/monk works for me.18:39
rick_h_ok, I was trying to pull from an existing merge request branch.18:39
rick_h_I'm probably doing more wrong, working on setting up something to be able to test with18:39
abentleyrick_h_: no, only a few branches get copied across.18:39
rick_h_abentley: ok, thanks18:40
abentleyrick_h_: so you need to push fresh ones, but that will work.18:40
rick_h_abentley: ok, will try that18:40
rick_h_abentley: any way to foce the make sync_branches on qa next?19:04
rick_h_or should I just be patient19:04
abentleyrick_h_: yes, ask a losa to run the appropriate script.19:04
jcsackettabentley: can i get a quick look at https://code.launchpad.net/~jcsackett/loggerhead/add-lock-icon/+merge/83203? it's very short.19:34
abentleyjcsackett: certainly.19:34
jcsackettthanks.19:35
abentleyjcsackett: r=me19:35
jcsackettthanks, abentley.19:35
deryckabentley, how are we doing on prepping deploy?  Just waiting on rollback to work it's way through now?19:43
abentleyderyck: No, https://code.launchpad.net/~benji/launchpad/bug-877195-code/+merge/80619 does not have enough information for me to do QA, and benji isn't around.19:43
deryckabentley, hmmm, perhaps one of his squad mates could help us? gary_poster or bac, any idea?19:46
abentleyrick_h_: You don't need to resubmit.  You can just push and tell me you've updated it.19:46
rick_h_abentley: updated MP per our discussions this morning: https://code.launchpad.net/~rharding/launchpad/bugfix_891735/+merge/8321719:46
rick_h_abentley: ah ok, it wasn't showing the changes after a push so I thought I had to resubmit it19:46
abentleyrick_h_: It should show that the diff is updating after a push, but it's easy to miss.19:46
bacderyck: let me look19:47
deryckbac, ok, thanks.  there are qa notes, but I'm guess abentley doesn't know the problem well enough to follow the notes.19:48
bacderyck: i just saw that19:48
gary_posterthanks bac19:51
abentleyrick_h_: bind_events doesn't do a lot.  Would it make sense to merge it into initializer?19:55
rick_h_abentley: it can. I tend to move bindings out for clarification and a home to group any/all in one place19:57
abentleyrick_h_: It's no biggie.  When I see a short single-call-site callable, I tend to move it to the callsite.19:58
rick_h_abentley: moved and pushed, no problem at all.19:59
abentleyrick_h_: On line 176, why set this._prev_scroll_height = this.get('min_height')?  Why not set it to the actual size, for example?20:00
sinzui_flacoste, ping20:01
flacostehi sinzui_20:01
rick_h_abentley: looking20:02
sinzui_flacoste, I *had* included the date of 2011-11-27 in the email to project owners to say when we would stop supporting shared private bugs...20:02
sinzui_flacoste, We are not technically stopping now, we are preventing users from creating new ones...20:03
abentleyrick_h_: Or perhaps allow it to be undefined, and let the first call to resize set it?20:03
flacostesinzui_: right20:03
rick_h_abentley: so if your textarea is 40px tall, but you set a min of 100, I need it to be different so resize kicks in20:03
rick_h_if I set it to undefined...I'd have to see what Math.max considers undefined20:04
sinzui_but I think I should mention that there will be a bug linking feature in the future and we expect to migrate the remaining shared bugs when the feature is available20:04
sinzui_^ flacoste20:04
abentleyrick_h_: this._prev_scroll_height is not an input to Math.max AFAICT.20:05
rick_h_abentley: ok, so tests pass with it at undefined20:05
rick_h_abentley: no, you're right, I was thinking the other way around. min_height20:05
abentleyrick_h_: Ah.20:05
rick_h_abentley: ok, removed it entirely, tests all pass with it set to default of 0 as a start point20:06
=== salgado is now known as salgado-afk
abentleyrick_h_: at 51, I think a second "var" would be clearer than having variable declaration spanning lines.20:07
flacostesinzui_: yes, that would be good20:08
sinzui_okay20:08
rick_h_abentley: ok, not a fan of the mutliple vars at a time?20:08
abentleyrick_h_: I'm fine with them if they don't span multiple lines.20:08
rick_h_oh heh, I'd hate them on one line. Ok,20:09
rick_h_updated20:09
abentleyrick_h_: at 53, please use Y.Lang.isUndefined or Y.Lang.isValue rather than !== undefined.20:10
abentleyrick_h_: Also, please indent only 4 spaces at 54.20:10
* rvba takes a look at qastaging to see if r14374 has been deployed.20:10
rvbaNot yet it seems…20:11
rick_h_abentley: sounds good20:12
abentleyrick_h_: Your comments have more lines at the bottom than I'd expect.  I'd expect just "*/" after the last line of text.20:13
rick_h_abentley: gotcha, sorry, the python docstring rules coming out in me20:13
abentleyrick_h_: Our docstrings have """ after the last line of text.20:14
rick_h_abentley: oh, could have sworn pep is one blank line at the end for readability. I missed that so far20:14
abentleyrick_h_: I haven't consulted the PEP, but I think that's how we always do it.20:15
rick_h_abentley: ok, thanks for the heads up. Looks like it's more a "suggestion" in the pep 257 for it "The BDFL [3] recommends inserting a blank line between the last paragraph in a multi-line docstring and its closing quotes"20:16
abentleyrick_h_: Interesting.20:17
rick_h_abentley: little retraining...to be expected :)20:17
abentleyrick_h_: please remove one newline around 172.20:17
=== matsubara is now known as matsubara-afk
rick_h_abentley: sure thing20:19
abentleyrick_h_: At 190, our style looks like this: http://pastebin.ubuntu.com/747478/20:19
abentleyrick_h_: i.e. if all the arguments don't fit on a line, we do a line break and indent.  This is not PEP8, but it's what we do.20:20
rick_h_abentley: ok, since that's still long I'd wrap to a thired line at the Math ok?20:20
rick_h_abentley: sure thing20:20
abentleyrick_h_: sure, that's fine.20:20
rick_h_http://pastebin.ubuntu.com/747480/ abentley so first/second preferred?20:21
rick_h_or doesn't matter20:21
abentleyrick_h_: first.20:21
rick_h_abentley: thanks20:21
abentleyAt 271, I've see it in other test suites, but I'm pretty dubious that saving two keystrokes is a win.20:22
rick_h_abentley: ?20:22
abentleyif you do "var Assert = Y.Assert;"  you can save two keystrokes by doing "Assert.areEqual" rather than "Y.Assert.areEqual".20:23
rick_h_abentley: ah, ok, yea I grabbed that out of other tests. Honestly, I'd prefer to just pull in the tests themselves and avoid the lookup costs of going through the object20:24
abentleyrick_h_: Anyhow.  No big deal either way.20:25
rick_h_abentley: no problem, find and replace is my friend.20:25
rick_h_updated and pushed20:26
abentleyrick_h_: 341, why not Assert.areSame?20:27
abentleyrick_h_: test_removing_content looks sensitive to browser settings, but maybe that's inevitable.20:29
rick_h_abentley: no reason, just inexperience with array of assert methods20:29
abentleyrick_h_: Ah.  Well, since we have it, let's use it.20:29
rick_h_abentley: how so? on the browser settings20:30
abentleyrick_h_: You're asserting that the height of test_text will be greater than 100 px, and "shrink" will be <= 100 px, but both seem to depend on font settings.20:31
rick_h_abentley: ah true, I tried to pick values that would not be drastic enough to change much, but perhaps I can do better with finding smaller test values for those20:32
abentleyrick_h_: I don't think it matters in a practical sense.20:33
abentleyrick_h_: Oh, 273 is excessively long.  That should be split up into a list and joined: https://dev.launchpad.net/JavaScriptReviewNotes#Plain_Old_JavaScript20:34
abentleyrick_h_: have you run make lint on this?20:35
rick_h_abentley: I've run jslint myself, I've not found the lint tool run during testing20:36
rick_h_abentley: so I was going to ask on the long block of text, would it be better to put into the test html and pull it out vs in the JS?20:36
abentleyrick_h_: Sorry, don't understand "not found the lint tool run during testing"20:36
rick_h_I've seen refernce to a lint tool that's run during the test/merge/etc process20:37
rick_h_abentley: no, I've not run lint on it20:37
abentleyrick_h_: right.  It's bin/lint.sh, but usually run as "make lint".20:37
abentleyrick_h_: I'd prefer to keep the long block of text in the JS.20:38
rick_h_abentley: ok20:38
abentleyrick_h_: at 379, "n" is too short as a variable name.  I suggest "node": https://dev.launchpad.net/JavaScriptReviewNotes#Plain_Old_JavaScript20:40
rick_h_abentley: ok20:41
abentleyrick_h_: please remove the newline at 453.  And I think that's everything.20:42
rick_h_abentley: can you give me some context around 453? I've been a few lines off and not sure where you mean20:43
abentleyrick_h_: I'm sorry.  443: http://pastebin.ubuntu.com/747500/20:46
abentleyrick_h_: we don't go for multiple blank lines except at module level, generally.20:47
rick_h_abentley: yes, my fault completely. Just wasn't looking at the filedupe file20:47
abentleyrick_h_: np.20:48
rick_h_abentley: thanks, changes all pushed20:48
abentleyrick_h_: This was far more thorough than reviews typically are, but since you'll want to get up to speed with our style, I indulged my inner nit-picker.20:49
rick_h_abentley: definitely, it's completely what I was expecting20:50
rick_h_though I had thought I tried to catch things a bit better, so sorry for all the issues20:50
rick_h_appreciate you letting me time sink your day20:50
rick_h_now I'll go get a drink and lick my wounds :)20:50
abentley:-)20:51
abentleyrick_h_: it was the same for me.20:51
abentleyrick_h_: r=me.20:55
rick_h_abentley: ty much sir21:10
pooliehi all22:00
poolieoh huw, your tags list is so soothing22:43
huwshimipoolie: :D22:44
huwshimipoolie: Looking forward to that being deployed :)22:56
poolieme too22:56
pooliehuwshimi one interesting thing is on eg https://bugs.qastaging.launchpad.net/bzr/+bugs it shows some tags with 022:57
pooliei guess they are official but there are no remaining open bugs22:57
pooliethis is reasonable22:57
wgrantpoolie: I believe that's always been the case. It just wasn't obvious before because the counts weren't shown.23:35
wgrantOoh23:35
wgrantBeta banner.23:35
jelmerwgrant: ooh, where?23:36
* jelmer is still wondering how to add that for mercurial imports23:37
wgrantjelmer: mmm, not sure how well it would work for imports. see eg. https://bugs.qastaging.launchpad.net/launchpad/+bugs23:37
jelmerwgrant: I don't see anything there23:37
wgrantYou may need to be in ~custom-buglisting-demo23:38
poolieyeah that is nice23:38
jelmerhmm, that looks good indeed23:52
jelmertakes up quite a bit more vertical space per bug than previously though23:52
wgrantYeah, and it's a lot less scannable.23:53
jelmerI think the beta banner would actually work for mercurial imports - we could show it on the branch page of imports, and just add a "(beta)" bit after the choice in the import creation menu.23:55
wgrantjelmer: True.23:55

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!