[15:02] <sinzui> Well
[15:02]  * sinzui stumbles his way to the chair
[15:03] <sinzui> == Agenda ==
[15:03] <sinzui>  * Roll call
[15:03] <sinzui>  * Next meeting
[15:03] <sinzui>  * Action items
[15:03] <sinzui>  * Queue status
[15:03] <sinzui>  * Mentoring update
[15:03] <sinzui>  * Review process
[15:03] <sinzui>   * (intellectronica) JS-related templates should be tested (as much as possible).
[15:03] <sinzui>   * (barry) don't assign GQ after on-call session, let next ocr take them
[15:03] <sinzui> Welcome to this week's AMEU reviewers meeting
[15:04]  * sinzui wonders if he is alone
[15:04] <salgado> me
[15:04] <intellectronica> me
[15:04] <bac> me
[15:04] <bigjools> me
[15:04] <salgado> you're not!
[15:04] <sinzui> me
[15:04] <allenap> me
[15:04] <BjornT> me
[15:05] <bigjools> I think we were waiting for the roll call!
[15:05] <bigjools> schwuk sends his apologies
[15:05] <sinzui> bigjools: so was I, yet I expect a few premature mes
[15:05] <bigjools> his wife is ill so he had to do the school run
[15:05] <sinzui> barry and gmb are excused
[15:06] <sinzui> we seem to be short a few people
[15:06] <bac> EdwinGrubbs: ping
[15:06] <flacoste> me
[15:06] <EdwinGrubbs> me
[15:07] <flacoste> danilos: ping
[15:07] <danilos> me
[15:07] <danilos> flacoste: thanks
[15:07] <sinzui> == Next meeting ==
[15:07] <sinzui> Today +7 days?
[15:07] <jtv> me
[15:08] <sinzui> Silence is compliance.
[15:09] <intellectronica> silence is golden
[15:09] <sinzui> Next meeting of the AMEU reviewers is in 7 days.
[15:09] <sinzui> == Agenda ==
[15:09] <sinzui>  * Roll call
[15:09] <sinzui>  * Next meeting
[15:09] <sinzui>  * Action items
[15:09] <sinzui>  * Queue status
[15:09] <sinzui>  * Mentoring update
[15:09] <sinzui>  * Review process
[15:10] <sinzui>   * (intellectronica) JS-related templates should be tested (as much as possible).
[15:10] <sinzui>   * (barry) don't assign GQ after on-call session, let next ocr take them
[15:10] <sinzui> bugger
[15:10] <sinzui> [15:10] <sinzui>  * intellectronica to file bug on lint issue regarding elementtree import
[15:10] <sinzui> I have not seen this gub
[15:10] <intellectronica> sorry, i haven't done it yet
[15:11] <sinzui> Shall I keep this item for next week?
[15:11] <sinzui>  * mwhudson to start discussion on page test purpose
[15:11] <sinzui> I have not seen this discussion. I look forward to it.
[15:12] <intellectronica> sinzui: please, yes
[15:12] <sinzui>  * thumper to submit a bug for moving ftests contents to tests
[15:12] <sinzui> /me has not see this bug either
[15:13] <sinzui>  * gmb to update PythonStyleGuide for long argument lists (use The Salgado Way)
[15:13] <sinzui> gmb did complete this
[15:13] <sinzui>  * barry to ask lifeless to summarize what he knows about the PQM Mysteries (e.g. autopacking bug losing branches)
[15:13] <sinzui> Not done.
[15:13] <sinzui>  * jml to find out how divmodders test their javascript
[15:13] <sinzui> Not done
[15:13] <sinzui> [15:14] <intellectronica> it's a nice thing to do if you can, but i think demanding it for every submission would be cruel
[15:14] <sinzui> schwuk: are you proposing a change to the cover letter in lpreview?
[15:14] <intellectronica> i think he's not here?...
[15:15] <sinzui> That is right
[15:15] <flacoste> yeah for large branch it makes sense
[15:15]  * sinzui drinks more coffee
[15:15] <flacoste> but i don't think making this mandatory is a good idea
[15:15] <sinzui> A summary of each file is nice, but I sometimes do it by feature/fix
[15:17] <sinzui> I think the reason we might want a summary by file is because there changes outside the feature/bug that were made, or the connection to the feature was non-obvious
[15:18] <sinzui> If that is the case, we might just remind the developer in the cover letter template that the non-obvious changes should be listed
[15:18] <jtv> "Go through your diff and..."
[15:18] <sinzui> That is what I do
[15:19]  * sinzui has written novellas.
[15:19] <bigjools> for big changes it really helps
[15:19] <intellectronica> we do have a cover letter template for non-trivial submissions
[15:20] <intellectronica> perhaps we should encourage people to you use it
[15:20] <intellectronica> ﻿me included, i don't i've used it more than once or twice
[15:20] <BjornT> i think a summary per file on works for certain type of changes. often, if you write it per-file, the cover letter won't flow as nicely as if you write one summary for all files.
[15:20]  * sinzui agrees with BjornT
[15:20] <bigjools> where certain type = big?
[15:21] <BjornT> having the modified files listed in the template would help with remembering non-related changes
[15:21] <bigjools> when I write my cover letters I usually have the diff open in another window to remind me what to say
[15:21] <sinzui> Changes that are subtle, spanning many files might not be big
[15:22] <bigjools> common sense should prevail
[15:22] <BjornT> bigjools: not necessarily. it's more like if you do non-related changes (which we shouldn't encourage people to do)
[15:22] <sinzui> There are also changes that cascade though tests and templates.
[15:23] <bigjools> it really depends on the change - it should be encouraged if it's a complicated change, otherwise it's not so useful
[15:24] <sinzui> Should regard listing changes as a best practice when the reason for the change are not obvious?
[15:24] <sinzui> s/should/Should we/
[15:25] <intellectronica> something like that, yes
[15:26] <sinzui> Do we document that on the wiki, or amend the cover letter template in review-submit
[15:27] <intellectronica> i don't think we should make this change in review-submit
[15:27]  * sinzui favours a wiki addition and an email to launchpad.
[15:27] <intellectronica> i can amend the cover letter template on the wiki, and we can remind devs that it is a good reference for an elaborate cover letter, when the situation calls for one
[15:28] <sinzui> I think that is a good start
[15:28] <schwuk> me
[15:28] <schwuk> sorry - got held up
[15:29] <sinzui> schwuk: we were concluding your proposal to Include file-by-file summary in review template
[15:29] <schwuk> sinzui: just been catching up
[15:31] <schwuk> sinzui: I think everything has been covered, so special to add. The per-file summary helps on large changes, but there should still be a general summary of the changes.
[15:31] <sinzui> schwuk: thank you. I agree
[15:32] <sinzui> == Queue status  ==
[15:32]  * sinzui refreshes the page
[15:32] <intellectronica> sinzui: ftr there's an action for me, above
[15:33] <intellectronica> sinzui: intellectronica to amend cover letter template to include a per-file summary section and write an email to the list
[15:33] <sinzui> intellectronica: thank you for reminding me of my responsabilities
[15:33] <sinzui> I will update the agenda for next week to include this
[15:34] <sinzui> I think the queue looks fine. mars needs to find a reviewer I think
[15:35] <mars> yep
[15:35] <sinzui> schwuk:  you have a missing branch in merge-conditional status.
[15:35] <intellectronica> i probably won't take any more reviews today after what i've got in the queue, so there may be some time on the gq at day's end
[15:35] <intellectronica> don't know about allenap
[15:35] <sinzui> schwuk: did you land it and remove the directory?
[15:36] <allenap> If I have time left, I'll look at the general queue for things to do.
[15:36] <schwuk> sinzui: possibly - I cleaned up branches on devpad when they were complaining about space
[15:37] <sinzui> salgado: you have broken up Mark's branch. Should it be in needs-review?
[15:37] <salgado> sinzui, no, I'll remove it from there
[15:37] <schwuk> sinzui: That branch was landed.
[15:38] <sinzui> schwuk: thank you
[15:38] <sinzui> Does anyone have anything else to say about the queue?
[15:39] <sinzui> ﻿== Mentoring update  ==
[15:39] <sinzui> Does anyone have something to say about this?
[15:40] <sinzui> == Review process ==
[15:40] <sinzui>   * (intellectronica) JS-related templates should be tested (as much as possible).
[15:40] <intellectronica> so
[15:40] <intellectronica> we don't test javascript code
[15:40] <intellectronica> but there's quite a lot we can do in our normal test suite
[15:41] <intellectronica> by testing the interaction between javascript and templates, for example
[15:41] <intellectronica> testing for the IDs of elements, testing for the presence of elements within a deeper structure, testing for the presence of a css class, etc'...
[15:42] <sinzui> testing that the script is present, that is contains the expected functions?
[15:42] <intellectronica> sinzui: indeed
[15:42] <intellectronica> also, make sure you run the ui locally and watch for javascript errors and the behaviour of javascript code
[15:43] <mars> and use FireBug ;)
[15:43] <intellectronica> finally, make sure you read javascript code with the same critical eye you read python code with
[15:43] <intellectronica> if you're unsure about how something works, ask questions
[15:43] <sinzui> firebug is not FF3 compatible
[15:43] <jtv> intellectronica: some browsers are also able to show warnings & errors in JS.  Maybe test with those?
[15:44]  * sinzui has fallen back to developer tools
[15:44] <intellectronica> think about defensive code. how can it fail? what will happen when it does? has everything been done to mitigate the effects of a failure
[15:44] <intellectronica> sinzui: there's an ubuntu package with a working firebug, mainained by asac
[15:45] <bac> i'm the self-appointed guardian angel of Safari.  if you review a branch with lots of JS changes let me know and i'll be glad to test the UI in Safari.
[15:45] <intellectronica> jtv: i think all browsers can show that, to some extent. but the most important thing is simply to look for breakage
[15:45] <mars> bac, cool, that will help
[15:45] <schwuk> sinzui: there's a beta of firebug that works with FF3
[15:45] <intellectronica> bac: great to know. i think allenap and mpt too have an apple close to hand
[15:45] <sinzui> My concern about testing for ids and classes, is that those might be variables in the script. So we are reduced to knowing that a script demands an id or class, and checking the browser.content
[15:46] <intellectronica> sinzui: perhaps we should add some helper functions to make it easier to test things in <script>
[15:46] <intellectronica> best is probably just to add such functions when the need arises, anyway
[15:46] <mpt> I don't usually bring my Mac to work any more
[15:47] <sinzui> intellectronica: I was thinking of extracting the getByID and $() to make a list of items to look for in the content
[15:47] <intellectronica> b.t.w it's possible to run both IE and Safari on ubuntu, if you're curious
[15:47] <sinzui> The mac is not important. Wenkit is
[15:47] <sinzui> WebKit is
[15:48] <sinzui> we can used the next gen of epiphany
[15:48] <intellectronica> anyway, i don't think we have too much trouble with platform/coverage
[15:48] <intellectronica> i think it's more a matter of reviewing the javascript code itself
[15:49] <bigjools> we're over time - is there much more?
[15:49] <intellectronica> and reviewing templates, which may be related to javascript code, even when it isn't obvious
[15:49] <sinzui> bigjools: one more item
[15:50] <mpt> sinzui, or Midori
[15:50] <mpt> (which has the advantage of being in the Hardy repository already)
[15:50] <sinzui> mpt: ! I did not know that
[15:51] <sinzui> intellectronica: I think we need a list of what to review. Checking for how/where a script connects to the page is doable by all of us.
[15:52] <intellectronica> sinzui: i'm happy to compile a list and put it on the wiki
[15:52] <sinzui> knowing when to use a library (like Mochikit) instead of doing writing your own block/function is another matter
[15:53] <mars> sinzui, yes, but that should be taken care of in the pre-implementation call, no?
[15:53] <intellectronica> sinzui: yeah, that could all be described as "is this the best way to write this feature"
[15:54] <intellectronica> not much different from python. it's just that we need to make sure we pay attention to this in the review
[15:54] <sinzui> intellectronica: Please compile the list, something is better than nothing. It will definitely help prevent a bug from some months ago that happened when the id the script required was not present.
[15:54] <intellectronica> and the one from last week ;)
[15:55] <intellectronica> sinzui: will do
[15:56] <sinzui> mars: it should, I think the issue is we do not have the JS experience in reviewing that we need given the script that has been written this year
[15:57] <sinzui> * (barry) don't assign GQ after on-call session, let next ocr take them
[15:57] <sinzui> barry is not here to explain his command. I intend to honor it.
[15:57] <sinzui> I don't have anything to say on this matter. If know one else has, I think this meeting is finished.
[15:57] <sinzui> 5
[15:57] <sinzui> 4
[15:57] <sinzui> 3
[15:57] <sinzui> 2
[15:57] <sinzui> 1
[15:57] <sinzui> So ends the meeting
[15:58] <intellectronica> thanks sinzui, great chairing
[15:58] <schwuk> Thanks sinzui
[15:58] <sinzui> Thank you all for playing
[15:58] <bigjools> thanks sinzui