[15:04] <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:05] <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:06] <EdwinGrubbs> noodles775: instead of "ping" do you say "Oi!"
[15:07] <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:08]  * 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:09] <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:10] <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:11] <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:12] <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:13] <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:14] <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:15] <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:16] <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:17] <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:18] <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:19] <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:20] <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:21] <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:22] <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:23] <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:24] <barry> bigjools: right, +1
[15:24] <bigjools> we were waiting for your branch to land so we could do that
[15:25] <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:26] <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: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: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 :)
[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.