| === bigjools-afk is now known as bigjools_ | ||
| === bigjools_ is now known as bigjools | ||
| === mrevell is now known as mrevell-lunch | ||
| === mrevell-lunch is now known as mrevell | ||
| === salgado_ is now known as salgado | ||
| barry | #startmeeting | 15:04 |
|---|---|---|
| henninge | Hey barry! | 15:04 |
| MootBot | Meeting started at 09:04. The chair is barry. | 15:04 |
| MootBot | Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] | 15:04 |
| barry | hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? | 15:04 |
| BjornT | me | 15:04 |
| abentley | me | 15:04 |
| henninge | me | 15:04 |
| bac | me | 15:04 |
| noodles775 | moi | 15:04 |
| sinzui | me | 15:04 |
| gary_poster | me | 15:04 |
| EdwinGrubbs | me | 15:05 |
| henninge | noodles775: toi? | 15:05 |
| flacoste | me | 15:05 |
| noodles775 | henninge: nah, it's just "Australish" | 15:05 |
| adeuring | me | 15:05 |
| intellectronica | me | 15:05 |
| henninge | ;-) | 15:05 |
| EdwinGrubbs | noodles775: instead of "ping" do you say "Oi!" | 15:06 |
| noodles775 | You got it Ezza ;) | 15:07 |
| barry | deryck, salgado cprov bigjools danilo-afk allenap mars ping | 15:07 |
| barry | [TOPIC] agenda | 15:07 |
| MootBot | New Topic: agenda | 15:07 |
| cprov | me | 15:07 |
| allenap | me | 15:07 |
| bigjools | meh | 15:07 |
| deryck | me, sorry | 15:07 |
| salgado | me | 15:07 |
| barry | * Roll call | 15:07 |
| barry | * Action items | 15:07 |
| barry | * UI review call update | 15:07 |
| barry | * new heading/title rules [barry] | 15:07 |
| barry | * 4-space indents for CSS styles [barry] | 15:07 |
| barry | * Peanut gallery (anything not on the agenda) | 15:07 |
| barry | 15:07 | |
| barry | 15:07 | |
| barry | [TOPIC] * Action items | 15:07 |
| MootBot | New Topic: * Action items | 15:07 |
| barry | [TOPIC] * Action items | 15:07 |
| MootBot | New Topic: * Action items | 15:07 |
| * barry thanks mootbot | 15:08 | |
| barry | * gary_poster and barry will transfer review guidelines from the old wiki and old old wiki to the new wiki | 15:08 |
| gary_poster | postponed to post 3.0 | 15:08 |
| barry | sorry, gary_poster asked me to get mrevell a list of pages to pull over but 3.0 is swamping me | 15:08 |
| danilos | me | 15:08 |
| barry | what gary_poster said :) | 15:08 |
| barry | * cprov to update guidelines to clarify how code sensitive to env changes should be written | 15:08 |
| cprov | oi! sorry I forgot about it. | 15:08 |
| barry | cprov: we'll just carry that one over. i'm hoping we'll be less stressed after this week or maybe next | 15:09 |
| barry | so no worries | 15:09 |
| cprov | barry: sure, thanks. | 15:09 |
| barry | [TOPIC] * UI review call update | 15:09 |
| barry | 15:09 | |
| MootBot | New Topic: * UI review call update | 15:09 |
| barry | let me see if beuno is around... | 15:10 |
| intellectronica | i think he's on holiday | 15:10 |
| barry | intellectronica: ah. | 15:10 |
| barry | right | 15:10 |
| intellectronica | one important thing we discussed is how overburdened ui reviewers are | 15:10 |
| barry | intellectronica: yes. and that this is an extra-ordinary time because of the push for 3.0 | 15:11 |
| intellectronica | to make it a bit easier until 3.0 is out, it's ok for people to land very simple changes (mechanical conversions and such) with ui=rs | 15:11 |
| barry | beuno did say he was going to start working with a few people to get them ui graduated | 15:11 |
| intellectronica | i think martin has writted to the list about that | 15:11 |
| barry | yep. intellectronica thanks | 15:11 |
| barry | that's all i can think of. intellectronica, anything else? | 15:12 |
| intellectronica | i think that's it | 15:12 |
| sinzui | Does everyone know that Blueprint pages are ui=rs? | 15:12 |
| noodles775 | Woops, I just landed the sprint-add mechanical change with r=rs and ui=rs... sorry. | 15:13 |
| barry | noodles775: right, still need a normal code review | 15:13 |
| noodles775 | Yep. | 15:14 |
| bigjools | yeehaw | 15:14 |
| intellectronica | sinzui: does that include volunteers? i bet there are many people out there itching to give us a hand with blueprint conversions. if that happens, can we land their changes without a proper ui review? | 15:14 |
| sinzui | noodles775: That is better than me. I was juggling 4 branches yesterday. I landed one by accident. I am glad bac approved it | 15:14 |
| noodles775 | heh | 15:14 |
| barry | i promised bac to land one thru ec2 and then promptly pqm-submitted it ;) | 15:15 |
| sinzui | intellectronica: I do not have an answer | 15:15 |
| barry | intellectronica: for volunteers, we still have to land the branch so i think a quick look couldn't hurt | 15:15 |
| flacoste | intellectronica, sinzui: if they are mechanical changes, yes | 15:15 |
| flacoste | i still think we should do a UI review for non-mechanical blueprints UI change | 15:15 |
| danilos | intellectronica, I've got 12 templates converted to generic-edit for blueprints, already reviewed, should land soon | 15:15 |
| flacoste | doesn't need to be much involved | 15:16 |
| danilos | intellectronica, i.e. make that 12 templates removed and replaced with generic-edit.pt | 15:16 |
| intellectronica | danilos: you're a star | 15:16 |
| sinzui | intellectronica: blueprint 2.0 pages will not work when we release, so doing a bad conversion job fixes more than will be broken in production | 15:16 |
| flacoste | there is only 12 unclaimed blueprints templates on https://dev.launchpad.net/VersionThreeDotO/BlueprintsConversion | 15:16 |
| barry | moving on.. | 15:17 |
| sinzui | The two blueprint pages I converted yesterday had whitespace issues. text overlapped text. I had to make some markup changes to make the page layout | 15:17 |
| barry | [TOPIC] * new heading/title rules [barry] | 15:18 |
| barry | 15:18 | |
| MootBot | New Topic: * new heading/title rules [barry] | 15:18 |
| barry | i want to make sure any question you have about the new rules get answered | 15:18 |
| danilos | barry, excellent | 15:18 |
| barry | i know that the breadcrumbs/titles/headers need some refinement, but that will have to wait until after 3.0. the rules we have now will not change until after 3.0 | 15:18 |
| barry | so... ask your questions now! :) | 15:19 |
| bigjools | plz barry just fix everythin kthxbye | 15:19 |
| noodles775 | barry: did you see deryck's email? | 15:19 |
| flacoste | that's actually a job for salgado :-) | 15:19 |
| deryck | barry, beuno, and I talked off list about breadcrumbs in titles a lot already | 15:19 |
| barry | noodles775: yes, but only in the sense that it made my inbox grow larger. i haven't read it yet. but deryck and i spoke this morning | 15:20 |
| noodles775 | Great. | 15:20 |
| barry | salgado is the breadcrumb man | 15:20 |
| deryck | I just replied to my own mail with the results of our call | 15:20 |
| barry | deryck: cool, thanks | 15:20 |
| deryck | np | 15:20 |
| danilos | ok, so, I am wondering about the h2/h1 issue... it seems the h2 with context.title is gone on most pages? is that expected? | 15:21 |
| danilos | or, how do we make it appear again? | 15:21 |
| barry | danilos: yes | 15:21 |
| noodles775 | Add view.label? | 15:21 |
| danilos | (sorry if I didn't pay close attention to the new rules page) | 15:21 |
| barry | noodles775: yes, it can be something like: | 15:21 |
| barry | @property | 15:21 |
| barry | def label(self): return self.context.label | 15:21 |
| danilos | noodles775, right, I am looking into figuring these out for all the translations page now that we've basically got everything converted | 15:21 |
| barry | noodles775: i would like to make that simpler (e.g. fall back to context.title) but not for 3.0 | 15:22 |
| noodles775 | Yep - that'd be great! | 15:22 |
| bigjools | we get rs=barry for fixing stuff like this I think you said? | 15:22 |
| intellectronica | barry: maybe have a mixin that does that? | 15:22 |
| barry | intellectronica: i think it can be done on LaunchpadView and/or base-layout.pt | 15:23 |
| barry | bigjools: sorry, what was the "stuff like this" in that sentence? | 15:23 |
| bigjools | barry: fixing existing 3.0 changed pages to conform to the new heading rules | 15:23 |
| barry | bigjools: right, +1 | 15:24 |
| bigjools | we were waiting for your branch to land so we could do that | 15:24 |
| barry | yep. now you can jfdi! :) | 15:25 |
| bigjools | thanks for the extra work :) | 15:25 |
| barry | :) | 15:25 |
| flacoste | actually, it's less work | 15:25 |
| flacoste | because otherwise, there were tweaks on a per-page basis | 15:26 |
| bigjools | for future changes, yes | 15:26 |
| flacoste | now, it's the same thing everywhere | 15:26 |
| barry | updating pagetests might not be fun | 15:26 |
| * flacoste mumble about approval tests | 15:26 | |
| barry | which is actually the main reason we aren't going to change the <title> rules until post 3.0 | 15:26 |
| * bigjools dreams of killing pagetitles.py | 15:27 | |
| sinzui | A lot of converted pages appear to be using pagetitle :( | 15:27 |
| barry | please everyone, take a healthy swing with a large baseball bat at pagetitles.py | 15:28 |
| sinzui | We'll know who cheated on day 1 of week 0 when pagetitles is deleted. | 15:28 |
| deryck | heh | 15:28 |
| * bigjools uses a cricket bat instead | 15:29 | |
| * sinzui uses a small cannonette | 15:29 | |
| barry | bigjools: just give it two healthy swings then | 15:29 |
| bigjools | :) | 15:29 |
| sinzui | um | 15:29 |
| barry | anything else on titles/headings/breadcrumbs? | 15:29 |
| barry | cool | 15:30 |
| sinzui | our designer is not available and I have not seen a design for the front page. Are we changing the page macro and declaring victory? | 15:30 |
| bigjools | while we're on the subject of UI, some of the two-column <dl>s are a bit borked if an item uses an icon | 15:31 |
| sinzui | leading icon? | 15:31 |
| * bigjools looks for an example | 15:32 | |
| bigjools | yes leading | 15:32 |
| bigjools | sinzui: https://edge.launchpad.net/ubuntu/+source/kdepim/ | 15:33 |
| bigjools | anyway, OT for this meeting I guess | 15:34 |
| barry | k, let's move on | 15:34 |
| barry | [TOPIC] * 4-space indents for CSS styles [barry] | 15:34 |
| MootBot | New Topic: * 4-space indents for CSS styles [barry] | 15:34 |
| barry | fairly simple one i think. most of our css in styles-3-0.css are 4 space indents | 15:34 |
| bigjools | JFDI | 15:35 |
| barry | any objections to keeping 4 space indents? | 15:35 |
| barry | 5 | 15:35 |
| barry | 4 | 15:35 |
| noodles775 | Nope - guessing it was just 2-spaces from pre-minimization days? | 15:35 |
| barry | 3 | 15:35 |
| sinzui | keeping it? I intentionally set it to 4, who put something else | 15:36 |
| barry | sinzui: dunno | 15:36 |
| noodles775 | sinzui: there's old stuff in style.css with 2 spaces I think. | 15:36 |
| barry | rs=me on fixing them for the next person to touch the file | 15:36 |
| barry | 4 spaces it is | 15:36 |
| barry | [TOPIC] peanut gallery | 15:36 |
| MootBot | New Topic: peanut gallery | 15:36 |
| sinzui | There should not be. stuff requires porting from specific CSS to generic | 15:36 |
| * bigjools has a peanut | 15:37 | |
| barry | bigjools: shuck it! | 15:37 |
| bigjools | ok | 15:37 |
| bigjools | real quick, but I sometimes see some obtuse page test code that does this: | 15:37 |
| sinzui | There should not be a zillion p.<class_that_is really_useful_but_can_only_be_used_by_paragrpahs> | 15:37 |
| bigjools | >>> browser.getLink("xxx").click() | 15:37 |
| bigjools | >>> print browser.url | 15:37 |
| bigjools | http://launchpad.dev/blah | 15:37 |
| abentley | bigjools: Then take some pictures of it, and you'll have your own peanut gallery. | 15:37 |
| bigjools | which causes an unnecessary page load | 15:37 |
| bigjools | you can just do: | 15:37 |
| bigjools | >>> print browser.getLink("xxx").url | 15:37 |
| bigjools | instead | 15:38 |
| bigjools | EOT | 15:38 |
| BjornT | bigjools: shouldn't the test make sure that the link leads to the right page? | 15:38 |
| barry | bigjools: oh, you mean if you aren't doing anything else on the target page? | 15:38 |
| bigjools | BjornT: they are the exact same test | 15:38 |
| sinzui | bigjools: barry: I think there is a misunderstanding here | 15:39 |
| bigjools | barry: yes | 15:39 |
| BjornT | bigjools: no. the first one makes sure the link isn't a 404. your version doesn't | 15:39 |
| bigjools | BjornT: if the test is doing nothing else, my version is better. Loading a page and doing nothing is a bad test. | 15:39 |
| sinzui | I do what bigjools is suggesting when I test that a view provides expected content, but a story is about link traversal | 15:39 |
| BjornT | bigjools: well, it still depends on the intention of the test | 15:40 |
| bigjools | it should ideally have a separate test for that page | 15:40 |
| bigjools | BjornT: yes, agreed | 15:40 |
| bigjools | but it's just something to look out for | 15:40 |
| bigjools | I am on the rampage against slow tests | 15:40 |
| bigjools | and this is low-hanging fruit for a big gain | 15:40 |
| sinzui | bigjools: most of the slow tests are pagetests that are checking the contract of a view. Covert them to a unittest or a doctest of the view. | 15:41 |
| barry | sinzui: +1 view tests rock | 15:41 |
| bigjools | yes, I think that's a separate issue to this though | 15:41 |
| bigjools | if another page test file tests the page, you don't need to load it somewhere else just to check a link exists | 15:42 |
| bigjools | anyway, that's it | 15:42 |
| sinzui | bigjools: right. That is why I argue we have far too many pagetests | 15:42 |
| barry | bigjools: cool thanks | 15:42 |
| barry | 3m left. anyone else? | 15:42 |
| BjornT | bigjools: well, if some other test makes sure you can navigate to that page, yes. that's what really is happening there. from page A we can reach page B | 15:43 |
| barry | okay, i think we're done. | 15:44 |
| barry | #endmeeting | 15:44 |
| MootBot | Meeting finished at 09:44. | 15:44 |
| noodles775 | Thanks barry! | 15:44 |
| barry | thanks everyone | 15:44 |
| deryck | thanks barry | 15:45 |
| bigjools | BjornT: I don't think you need to click through to ascertain that though | 15:45 |
| bigjools | cheers barry | 15:45 |
| bigjools | BjornT: if the other test opens the same URL | 15:45 |
| henninge | bigjools: I don't get the core of the discussion here. | 15:46 |
| BjornT | bigjools: again, it's testing that you can navigate to the page, not that you can open it. (and again, it depends on the intention of the test, which isn't always easy to know) | 15:46 |
| henninge | bigjools: are you saying that 'print browser.url' triggers a page load like 'click()' does? | 15:46 |
| bigjools | all you need to know is that the link exists | 15:47 |
| bigjools | henninge: no, it doesn't, that's the point | 15:47 |
| bigjools | if the link exists then you can assume it works | 15:47 |
| BjornT | bigjools: what if the link is a 404? | 15:47 |
| bigjools | *provided* you have another test that opens the same URL | 15:47 |
| BjornT | bigjools: the point with test is that you don't have to assume that things work, you can make sure ;) | 15:47 |
| bigjools | if you don't open the link anywhere else then yes, you must click it | 15:47 |
| BjornT | bigjools: and provided you keep the two tests in sync | 15:47 |
| bigjools | that's easy enough | 15:48 |
| BjornT | bigjools: it is? how? | 15:48 |
| bigjools | but it also separates tests in a nicer way | 15:48 |
| bigjools | both tests will fail if you change the URL | 15:48 |
| sinzui | bigjools: I don't think this is an issue is we separated the test of the view's contracts and expected out put from the simple story of link traversal | 15:49 |
| sinzui | s/is/if/ | 15:49 |
| BjornT | bigjools: not necessarily. what if the page name is hardcoded in the template? | 15:49 |
| bigjools | BjornT: I don't understand the point? | 15:49 |
| BjornT | bigjools: page A links to page B (having a <a href="B">B</a> in the page. you have a test that opens B. | 15:50 |
| BjornT | bigjools: what happens if you rename B to C, forgetting to change the link in page A? | 15:50 |
| bigjools | nothing changes from the scenario I posted in the meeting | 15:51 |
| bigjools | it would fail in the same way | 15:51 |
| bigjools | ie not fail :) | 15:51 |
| BjornT | bigjools: not. the original version would fail. your version wouldn't. | 15:51 |
| BjornT | bigjools: click() fails if it goes to a 404 | 15:52 |
| bigjools | yes, but we always redirect from old URLs. if we don't then that's bad | 15:52 |
| bigjools | but I see your point | 15:52 |
| BjornT | bigjools: my main point is that it's not always a good idea to fix those cases. pagetests should test workflows, not single pages | 15:52 |
| BjornT | bigjools: there are many cases where we don't redirect from old URLs | 15:53 |
| bigjools | I still think we need to avoid extra page loads | 15:53 |
| bigjools | they are expensive | 15:53 |
| bigjools | how often do we change URLs? | 15:53 |
| BjornT | bigjools: you're suggesting we should stop testing for things we only change rarely? | 15:54 |
| sinzui | bigjools: What is wrong with my suggestion to separate content testing in a view test from a browser test that checks links? | 15:54 |
| bigjools | sinzui: nothing, it's great | 15:54 |
| bigjools | BjornT: rarely, or never? | 15:54 |
| BjornT | bigjools: both. there's no such thing as 'code that will never change' ;) | 15:55 |
| bigjools | BjornT: have you looked at Soyuz lately? :) | 15:55 |
| bigjools | </joke> | 15:55 |
| BjornT | bigjools: my main point is, we shouldn't blindly reduce test coverage to make things faster | 15:56 |
| bigjools | BjornT: it's a case-by-case consideration, I understand your point and agree, but I think that in some cases the test is slow for no reason | 15:56 |
| BjornT | bigjools: right, i agree, it's a case-by-case consideration, that was where i was going | 15:56 |
| bigjools | so we agree in our agreement :) | 15:57 |
| === danilos is now known as danilos-afk | ||
| === salgado is now known as salgado-lunch | ||
| === salgado-lunch is now known as salgado | ||
| === cprov is now known as cprov-afk | ||
| === salgado is now known as salgado-afk | ||
| barry | #startmeeting | 23:30 |
| MootBot | Meeting started at 17:30. The chair is barry. | 23:30 |
| MootBot | Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] | 23:30 |
| barry | hi guys | 23:30 |
| rockstar | ni! | 23:30 |
| wgrant | me | 23:31 |
| thumper | hi | 23:31 |
| barry | jml, mwhudson ping | 23:31 |
| jml | hi | 23:31 |
| barry | so. let's start with a recap of ameu | 23:32 |
| mwhudson | hi | 23:32 |
| barry | all updates to blueprint pages and all mechanical changes are ui=rs | 23:32 |
| barry | they still need a code review though | 23:32 |
| barry | wgrant: that includes volunteers :) | 23:32 |
| mwhudson | blueprints? what's that again? :) | 23:32 |
| barry | :D | 23:32 |
| barry | use 4 space indents in css files | 23:33 |
| wgrant | barry: Tempting, tempting... | 23:33 |
| barry | that's it! | 23:33 |
| mwhudson | cool, short and sweet | 23:33 |
| barry | thumper: has something for today... | 23:33 |
| rockstar | thumper landed a new toy today. He should talk about it. | 23:33 |
| thumper | :) | 23:34 |
| thumper | r9475 of devel has a new method on TestCase | 23:34 |
| thumper | def assertStatementCount(self, expected_count, function, *args, **kwargs): | 23:34 |
| thumper | it uses the StormStatementRecorder | 23:34 |
| thumper | (also in r9475) | 23:34 |
| thumper | you need to be careful about the state of the store cache | 23:34 |
| jml | thumper, wow, that's a great idea :P | 23:34 |
| thumper | jml: :) | 23:35 |
| thumper | but it is good for asserting that you aren't hitting the DB when you don't expect to | 23:35 |
| mwhudson | thumper: can you use the StormStatementRecorder to find out more about the actual queries than just the count? | 23:35 |
| rockstar | thumper, can we find a way to get the information from StormStatementRecorder into every HTTPResponse for our dev server? | 23:35 |
| mwhudson | although i'm having a hard time thinking about why you'd want that, now i've said it... | 23:36 |
| thumper | mwhudson: if the count doesn't equal, it prints out the actual statements in the fail message | 23:36 |
| barry | thumper: when you say "you need to be careful about the state of the store cache" what does that mean? | 23:36 |
| mwhudson | thumper: cool | 23:36 |
| thumper | barry: if you are creating objects in the test, then they are in the cache | 23:36 |
| thumper | barry: so factory.makeBranch will already have the branch owner in the cache | 23:36 |
| thumper | where as if you have just loaded the branch itself, it won't be | 23:36 |
| thumper | so | 23:37 |
| thumper | you need to do something like: | 23:37 |
| thumper | store = Store.of(obj) | 23:37 |
| thumper | store.flush() | 23:37 |
| thumper | store.reset() | 23:37 |
| thumper | reload the obj somehow using a utility | 23:37 |
| thumper | then assert counts | 23:37 |
| thumper | the store.reset() removes the hidden storm store attribute of the object | 23:37 |
| thumper | so you can't just use: store.reload(obj) | 23:38 |
| thumper | it doesn't work | 23:38 |
| barry | thumper: this sounds cool even today when i'm more fully awake :) please email launchpad-dev to let people know about it and/or add it to our dev wiki | 23:38 |
| thumper | this _maybe_ solvable with enough storm-fu | 23:38 |
| rockstar | thumper, I think the landscape guys might like to know about it too. | 23:38 |
| thumper | perhaps | 23:39 |
| thumper | I'm using it to confirm that my "priming of the storm cache" actually does what I expect | 23:39 |
| barry | thumper: this is cool, thanks for adding it, and please do let everyone know about | 23:40 |
| thumper | ok | 23:41 |
| barry | anything else guys? | 23:41 |
| rockstar | Not from me. | 23:41 |
| thumper | that's it from me | 23:41 |
| barry | wgrant, jml, mwhudson ? | 23:41 |
| jml | not from me | 23:41 |
| mwhudson | nope | 23:42 |
| barry | great, thanks everyone! | 23:42 |
| wgrant | no | 23:42 |
| barry | #endmeeting | 23:42 |
| MootBot | Meeting finished at 17:42. | 23:42 |
| * rockstar takes the dog for a walk. | 23:42 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!