=== 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 [15:04] #startmeeting [15:04] Hey barry! [15:04] Meeting started at 09:04. The chair is barry. [15:04] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [15:04] hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? [15:04] me [15:04] me [15:04] me [15:04] me [15:04] moi [15:04] me [15:04] me [15:05] me [15:05] noodles775: toi? [15:05] me [15:05] henninge: nah, it's just "Australish" [15:05] me [15:05] me [15:05] ;-) [15:06] noodles775: instead of "ping" do you say "Oi!" [15:07] You got it Ezza ;) [15:07] deryck, salgado cprov bigjools danilo-afk allenap mars ping [15:07] [TOPIC] agenda [15:07] New Topic: agenda [15:07] me [15:07] me [15:07] meh [15:07] me, sorry [15:07] me [15:07] * Roll call [15:07] * Action items [15:07] * UI review call update [15:07] * new heading/title rules [barry] [15:07] * 4-space indents for CSS styles [barry] [15:07] * Peanut gallery (anything not on the agenda) [15:07] [15:07] [15:07] [TOPIC] * Action items [15:07] New Topic: * Action items [15:07] [TOPIC] * Action items [15:07] New Topic: * Action items [15:08] * barry thanks mootbot [15:08] * gary_poster and barry will transfer review guidelines from the old wiki and old old wiki to the new wiki [15:08] postponed to post 3.0 [15:08] sorry, gary_poster asked me to get mrevell a list of pages to pull over but 3.0 is swamping me [15:08] me [15:08] what gary_poster said :) [15:08] * cprov to update guidelines to clarify how code sensitive to env changes should be written [15:08] oi! sorry I forgot about it. [15:09] cprov: we'll just carry that one over. i'm hoping we'll be less stressed after this week or maybe next [15:09] so no worries [15:09] barry: sure, thanks. [15:09] [TOPIC] * UI review call update [15:09] [15:09] New Topic: * UI review call update [15:10] let me see if beuno is around... [15:10] i think he's on holiday [15:10] intellectronica: ah. [15:10] right [15:10] one important thing we discussed is how overburdened ui reviewers are [15:11] intellectronica: yes. and that this is an extra-ordinary time because of the push for 3.0 [15:11] 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] beuno did say he was going to start working with a few people to get them ui graduated [15:11] i think martin has writted to the list about that [15:11] yep. intellectronica thanks [15:12] that's all i can think of. intellectronica, anything else? [15:12] i think that's it [15:12] Does everyone know that Blueprint pages are ui=rs? [15:13] Woops, I just landed the sprint-add mechanical change with r=rs and ui=rs... sorry. [15:13] noodles775: right, still need a normal code review [15:14] Yep. [15:14] yeehaw [15:14] 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] 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] heh [15:15] i promised bac to land one thru ec2 and then promptly pqm-submitted it ;) [15:15] intellectronica: I do not have an answer [15:15] intellectronica: for volunteers, we still have to land the branch so i think a quick look couldn't hurt [15:15] intellectronica, sinzui: if they are mechanical changes, yes [15:15] i still think we should do a UI review for non-mechanical blueprints UI change [15:15] intellectronica, I've got 12 templates converted to generic-edit for blueprints, already reviewed, should land soon [15:16] doesn't need to be much involved [15:16] intellectronica, i.e. make that 12 templates removed and replaced with generic-edit.pt [15:16] danilos: you're a star [15:16] 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] there is only 12 unclaimed blueprints templates on https://dev.launchpad.net/VersionThreeDotO/BlueprintsConversion [15:17] moving on.. [15:17] 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:18] [TOPIC] * new heading/title rules [barry] [15:18] [15:18] New Topic: * new heading/title rules [barry] [15:18] i want to make sure any question you have about the new rules get answered [15:18] barry, excellent [15:18] 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:19] so... ask your questions now! :) [15:19] plz barry just fix everythin kthxbye [15:19] barry: did you see deryck's email? [15:19] that's actually a job for salgado :-) [15:19] barry, beuno, and I talked off list about breadcrumbs in titles a lot already [15:20] 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] Great. [15:20] salgado is the breadcrumb man [15:20] I just replied to my own mail with the results of our call [15:20] deryck: cool, thanks [15:20] np [15:21] 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] or, how do we make it appear again? [15:21] danilos: yes [15:21] Add view.label? [15:21] (sorry if I didn't pay close attention to the new rules page) [15:21] noodles775: yes, it can be something like: [15:21] @property [15:21] def label(self): return self.context.label [15:21] noodles775, right, I am looking into figuring these out for all the translations page now that we've basically got everything converted [15:22] noodles775: i would like to make that simpler (e.g. fall back to context.title) but not for 3.0 [15:22] Yep - that'd be great! [15:22] we get rs=barry for fixing stuff like this I think you said? [15:22] barry: maybe have a mixin that does that? [15:23] intellectronica: i think it can be done on LaunchpadView and/or base-layout.pt [15:23] bigjools: sorry, what was the "stuff like this" in that sentence? [15:23] barry: fixing existing 3.0 changed pages to conform to the new heading rules [15:24] bigjools: right, +1 [15:24] we were waiting for your branch to land so we could do that [15:25] yep. now you can jfdi! :) [15:25] thanks for the extra work :) [15:25] :) [15:25] actually, it's less work [15:26] because otherwise, there were tweaks on a per-page basis [15:26] for future changes, yes [15:26] now, it's the same thing everywhere [15:26] updating pagetests might not be fun [15:26] * flacoste mumble about approval tests [15:26] which is actually the main reason we aren't going to change the rules until post 3.0 [15:27] * bigjools dreams of killing pagetitles.py [15:27] <sinzui> A lot of converted pages appear to be using pagetitle :( [15:28] <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:29] * 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:30] <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:31] <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:32] * bigjools looks for an example [15:32] <bigjools> yes leading [15:33] <bigjools> sinzui: https://edge.launchpad.net/ubuntu/+source/kdepim/ [15:34] <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:35] <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:36] <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:37] * 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:38] <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:39] <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:40] <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:41] <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:42] <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:43] <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:44] <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:45] <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:46] <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:47] <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:48] <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:49] <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:50] <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:51] <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:52] <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:53] <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:54] <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:55] <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:56] <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:57] <bigjools> so we agree in our agreement :) === 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 [23:30] <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:31] <wgrant> me [23:31] <thumper> hi [23:31] <barry> jml, mwhudson ping [23:31] <jml> hi [23:32] <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:33] <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:34] <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:35] <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:36] <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:37] <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:38] <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:39] <thumper> perhaps [23:39] <thumper> I'm using it to confirm that my "priming of the storm cache" actually does what I expect [23:40] <barry> thumper: this is cool, thanks for adding it, and please do let everyone know about [23:41] <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:42] <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.