=== rocksta`` is now known as rockstar === salgado-afk is now known as salgado === bigjools-afk is now known as bigjools [15:00] #startmeeting [15:00] Meeting started at 09:00. The chair is barry. [15:00] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [15:00] hello everyone and welcome to this week's ameu reveiwers meeting. who's here today? [15:00] me [15:00] me [15:00] me [15:00] me [15:00] me [15:00] me [15:00] me [15:00] me [15:01] me [15:01] me [15:01] me [15:02] me [15:02] BjornT: ping [15:02] cprov: ping [15:02] danilos: ping [15:02] EdwinGrubbs: ping [15:02] me [15:02] me [15:02] me [15:03] rockstar: ping [15:03] sinzui: ping [15:03] me [15:03] i think that's everyone [15:03] [TOPIC] agenda [15:03] New Topic: agenda [15:03] * Roll call [15:03] * Action items [15:03] * Mentoring update [15:03] * Peanut gallery (anything not on the agenda) [15:03] * naming of unittest methods (Camel``Case or under_scores) (henninge) [15:03] * order of assert parameters (expected, observed) (hennige) [15:03] * Driving maintainable Javascript code (intellectronica) [15:03] * Modularization and unit testing are strongly encouraged [15:03] * Refactoring code after a UI review is often a good idea [15:03] * The upper limit for JS diffs is 1600 lines (because javascript is so verbose) [15:03] [TOPIC] action items [15:03] New Topic: action items [15:04] i think the only one outstanding is gary's and he's away today [15:04] so... [15:04] [TOPIC] mentoring update [15:04] New Topic: mentoring update [15:04] i think we have just one mentored reviewer left, and noodles isn't here today [15:04] so... [15:05] [TOPIC] peanut gallery [15:05] New Topic: peanut gallery [15:05] * naming of unittest methods (Camel``Case or under_scores) (henninge) [15:05] * order of assert parameters (expected, observed) (hennige) [15:05] henninge: the floor is yours [15:05] Hi [15:05] Yesterday cprov and I ran into this discussion about how to name unittest methods. [15:06] I have always done test_the_frobnob, while his branch had testTheFrobnob [15:06] The argument is: Coding style for methods is CamelCase [15:06] yes, and I've used CamelCase for most of the soyuz unittest, and it's not looking good. [15:07] isn't the policy to use test_methodName()? [15:07] i have a feeling of deja vu [15:07] * barry would greatly prefer anything make makes us more pep 8-y [15:07] salgado: exactly [15:07] I thought unit_tests_were_the_exception? [15:07] but these methods are never called directly and only appear in test runner [15:07] output [15:07] salgado: +1 [15:07] this was changed a few months back, but I don't remember what it was changed to exactly. [15:07] barry: +1 [15:07] we already discussed that [15:08] the convention for unit test is: [15:08] test_methodName_with_particular_condition [15:08] flacoste: that's what i remember too. [15:08] ah, that was my faint memory, too. [15:08] we had this discussion like 6-8 months ago [15:08] flacoste: yes [15:08] When testing a specific Launchpad method, a mix of PEP 8 and camel case is [15:08] used, e.g. test_fooBarBaz() [15:08] is it not in the style guide? [15:08] barry: somebody should have updated the wiki ;-) [15:09] https://dev.launchpad.net/TestsStyleGuide#Functional and Unit Tests [15:09] flacoste, it's there! [15:09] In general, you should follow Launchpad coding conventions (see PythonStyleGuide), however when naming test methods: [15:09] ... [15:09] perfect! [15:09] flacoste: time machine activated! [15:09] * bigjools sees a rapid end to the discussion coming [15:09] anybody wants to rehash this? [15:09] cool, problem solved. [15:09] second item [15:10] I have seen that LaunchpadTestCase uses assert(expected, observed) [15:10] and have used the asserts that way. [15:10] But I see a lot of (obseved, expected). [15:11] henninge: +1 for (expected, observed) [15:11] barry: me, too [15:11] do you mean assertEqual? [15:11] is there a guide line for that already? [15:11] i don't understand [15:11] bigjools: yeah [15:11] oh sorry, I meant assert*(e,o) [15:12] btw, I found out that not everybody knows about LaunchpadTestCase [15:12] intellectronica, self.assertEquals(sut_output, 'what I expect') instead of self.assertEquals('what I expect', sut_output) [15:12] henninge: could you please update https://dev.launchpad.net/TestsStyleGuide [15:12] i don't think it's worth having a policy on that. it doesn't matter [15:13] I don't either [15:13] agreed [15:13] actually, i think it's better to be consistent because it's easier to debug [15:13] how does it make it easier? [15:13] bigjools: you have to think less when you see failing output [15:14] yes, it's for clarity. [15:14] if you want to make it really easy you can do `expected = expr \n observed = expr \n assertEqual(expected, observed)` [15:15] intellectronica: true [15:15] yeah, I quite like that style [15:15] that is fine by me, too. [15:15] intellectronica: it would be better to have that in the framework [15:15] but again, i don't think we should force this. just remember that it's a possibility [15:15] I think encouraged but not enforced [15:15] just sayin', all things being equal, we prefer (expected, observed) [15:15] bigjools: +1 [15:16] bigjools: +1 [15:16] bigjools: +1 [15:16] * bigjools is not used to this many people agreeing with him [15:16] henninge: can you capture this in the wiki? [15:16] barry: will do [15:17] [ACTION] henninge to capture decision on assert*(expected, observed) preference in wiki [15:17] ACTION received: henninge to capture decision on assert*(expected, observed) preference in wiki [15:17] henninge: thanks! [15:17] np [15:17] [TOPIC] * Driving maintainable Javascript code (intellectronica) [15:17] New Topic: * Driving maintainable Javascript code (intellectronica) [15:17] * Modularization and unit testing are strongly encouraged [15:17] * Refactoring code after a UI review is often a good idea [15:17] * The upper limit for JS diffs is 1600 lines (because javascript is so verbose) [15:17] what a stupid title. i should be shot [15:17] intellectronica: why? because of the oxymoron? :) [15:18] * bigjools chuckles [15:18] intellectronica: the floor is yours [15:18] in the last ajax swat team meeting, we rockstar and deryck led a discussion about how to produce better (more maintainable, easier to test) javascript code [15:18] we made several observations: [15:19] 1. code that is OO and modularized is much easier to work with than code that is very imperative and/or relies on closure too heavily [15:19] 2. unit tests (aka YUI tests) are a great way to test our code. in many cases much better than integration tests [15:20] 3. working on UI features it is often easier to first concentrate on completing a working UI than on code quality [15:20] that's fine, but it means that we should refactor the code immediately after it passes UI review [15:20] that can happen either before code review, or if not, the reviewer should encourage doing that (either as part of this branch or in a followup branch) [15:21] finally, since javascript code is so much more verbose, we propose a much higher limit, in order to encourage developers to not skip these important steps [15:21] one of those being writing unit tests [15:21] we think 1600 lines (double the limit we have for python) is reasonable [15:22] intellectronica: how can we get more assistance in making our js work correctly, and be more consistent? although everyone i've asked for help has been very helpful, i still often feel like i'm just hacking in the dark until it works [15:22] just unit tests can talk 2/3 of your branch size [15:22] intellectronica: when you say "javascript unit test" you're not talking about windmill, right? [15:22] or are you? [15:23] barry: to some extent, we are all a bit in the dark, discovering how to best go about this. discussions in #rhinos and on the canonical-javascript mailing list are a good way to exchange ideas [15:23] barry, yuitest, not windmill [15:23] barry: no. we're talking about in-page unit tests using the yuitest framework [15:23] intellectronica: fair enough :) [15:23] you'll be glad to learn that they are much easier to write and execute [15:23] mars: i don't know anything about yuitest. where can i learn and/or where are good examples in our code? [15:24] lazr-js/src/*/tests [15:24] intellectronica: that would make me very very happy :) [15:24] where are these unit tests in our tree? [15:24] barry: see lib/canonical/launchpad/javascript/soyuz/tests for example [15:24] barry, right now the best examples are in lazr-js itself, but others will be landing examples in launchpad soon [15:24] <...>/javascript/*/tests [15:24] also all widgets in lazr-js have tests [15:24] currently,only the soyuz module has some [15:24] yep lazr-js/src/*/tests [15:24] cool, thanks, i will be looking at those for the branch i'm currently working on [15:25] unit tests are really "unit" tests [15:25] no Launchpad [15:25] only DOM and you code [15:25] and it's an self-created DOM [15:25] barry, btw, the test infrastructure is open for hacking and improvement. Please feel free to stop the pain :) [15:25] So I believe lp_dynamic_dom_updater.js is the only Launchpad test in our tree [15:25] in lauchpad, yes [15:25] lazr-js has tons of them [15:25] so you still need to do some integration testing, but the advantage is that they encourage you to do a good job on modularizing your code [15:26] mars: i will as soon as i can localize it. right now, it's mostly a full-body numbing throb. :) [15:27] i think the other problem is that anything touching js just takes WAY more time to develop. i don't think it's entirely the fault of not being as familiar with js. it's a function of the ui 90/10 rule and the fact that yui documentation is lousy [15:27] * barry knows he's just ranting [15:28] barry: that, and the fact that we're all still learning. i bet in a couple of years we'll all be js/yui wizards. it takes time to build competency [15:28] barry, thankfully we have started to work on that. intellectronica's process improvements are an important step towards speeding things up [15:28] intellectronica: true. though, we need to keep that in mind when estimating story points and such [15:29] intellectronica: so your saying in a couple of year javascript will require less keystrokes to code? [15:29] mars: yes, and they are greatly appreciated. our velocity will certainly improve over time [15:29] sinzui, yes, actually [15:29] * beuno yells from the back "FASTER! FASTER!" [15:29] sinzui: i'm sure in a couple of years you will have written enough gedit plugins to automate most of the crud :) [15:29] I guess the soyuz team won't notice since their class names exceed the line limit [15:30] * barry mumbles "dabbrev" [15:30] * sinzui pines for Javascript 1.7/ECMA ?? [15:30] at least we have tests! [15:30] sinzui, you want ECMA 3.1 :) [15:30] i think we digress. maybe we should start a js coder support group for ranting... [15:30] is everyone cool with the increase in diff limit (and understands why it's necessary)? [15:30] intellectronica: yes :) [15:31] I think YUI looks to Java for inspirtation where are Javascript 1.7 looks to Python [15:31] sinzui, not the cloaked OO pining of 4.X ;) [15:31] intellectronica: it does make it more difficult to review, but it does make sense. [15:31] barry: well, you don't really need to review the block delimiters and the uber-verbose documentation [15:32] intellectronica, +1 [15:32] intellectronica: true [15:32] intellectronica: i'm certainly up for an experiment here. if it's too painful we can address it again [15:33] i think i'm done, unless anyone has anything to add [15:33] and no, "javascript sucks" isn't a valid item. we already know that :-/ [15:33] intellectronica: thanks. can you send a message to launchpad@ about the length limit? [15:33] :P [15:33] barry: will do [15:34] [ACTION] intellectronica to email launchpad@ about js branch length limit increase [15:34] ACTION received: intellectronica to email launchpad@ about js branch length limit increase [15:34] How are we increasing our JS reviewers? [15:34] sinzui: i think by now pretty much everyone on the team does js reviews [15:34] sinzui, formally? [15:34] which team? [15:35] bigjools: reviewers team (which is almost the same as the developers team) [15:35] intellectronica: I don't review JS then [15:35] sinzui, because js reviews are open to all, we leave it up to the reviewer's discretion [15:35] mars: I know a lot about javascript, but I do not consider myself a review because I do not know much about YUI, YUI unit tests, and windmill [15:35] right. we're asking all reviewers to do js reviews, but ask the swat team for help when needed [15:35] bigjools: well, time to start, then :) [15:35] intellectronica: I need to write some js myself first :) [15:35] same with me [15:36] me2 [15:36] bigjools: yes, i guess that's a prerequisite [15:36] one problem with this though is that there's little consistency in our js style [15:36] sinzui, right. Depends on the review. If it is a one-line logic bug, then you don't need a JS mentor call [15:36] for example, every branch i've written has had different recommendations during js review [15:36] barry: anything in particular you've noticed? [15:37] sinzui, but if you desire a second opinion, then give someone on the AJAX team a shout [15:37] intellectronica: little things mostly, like where to stash objects, anim-reversing [15:37] intellectronica: i think of it more as refinement rather than conflicts [15:37] yeah [15:37] that pattern are not clear yet [15:38] mars: can you remind us who is on the "ajax team"? [15:38] anim-reversing is about learning, and weak docs (my fault) [15:38] so i think it's normal to get divergent opinion on these aspects [15:38] mars: no, i think flacoste has it right. we're learning as we go, so consistency suffers for now. we're just building up tech debt we know we'll have to pay off later [15:38] * barry sees no other way around that [15:38] barry: EdwinGrubbs, mars, intellectronica, jtv, flacoste, rockstar [15:39] barry: and noodles [15:39] * mars can't type today [15:39] and danilos - by virtue of having attended the Berlin sprint [15:39] flacoste: thanks. we should capture that on a wiki page, if it isn't already [15:39] barry: I think it is [15:39] barry, it is [15:39] cool [15:39] thanks [15:39] barry: deryck too knows a lot about js and yui and can help, b.t.w [15:40] barry, just check the 'skills' column on the current reviewers page [15:40] barry: https://dev.launchpad.net/ReviewerSchedule [15:40] s/skills/specialities/ [15:40] as i said, everytime i've asked someone for some js help, y'all have been very nice and very helpful. thanks! [15:41] we have 5 minutes left. is there anything else to talk about today? [15:41] yes [15:41] i have [15:41] i'd like to get an update on the JS testing experiment [15:41] anyone tried it out in the last week? [15:42] nope [15:42] no, no JS landings :( [15:42] really? [15:42] ok [15:43] a better question is: anyone been through review with a js branch which hasn't gone through testing? [15:43] yes, that's the proper way to phrase the question? [15:43] ! [15:43] no, but i have a js branch going into review hopefully today [15:43] aha! [15:43] i guess i can be a guinea pig :) [15:44] what about the timeline branches that Edwin landed last week? [15:45] flacoste, the easiest way to push back on this may be to have reviewer's ask "Has it been tested on all browsers?" [15:46] guess not. we're outta time, so shall we call it a day? [15:46] sure [15:46] flacoste: let's ask again next week! [15:46] #endmeeting [15:46] Meeting finished at 09:46. [15:46] thanks everyone [15:46] thanks barry [15:46] thanks barry [15:46] Ta barry. === salgado is now known as salgado-afk === Edwin is now known as Guest37223 [23:29] lalalala [23:29] hi [23:30] hi ho [23:30] #startmeeting [23:30] Meeting started at 17:30. The chair is barry. [23:30] Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] [23:30] jml, mwhudson, thumper hi [23:30] hi [23:31] hi [23:31] hello [23:31] so, do you guys have anything or would you like me to start by reviewing the summary from ameu? [23:31] summary is good [23:31] summarizing is good [23:32] [TOPIC] summary from ameu [23:32] New Topic: summary from ameu [23:32] henninge brought up a question about the order of parameters in unittest assert*() methods [23:32] we agreed that we "prefer" but won't "enforce" the order: (expected, observed) [23:32] +1 [23:32] +1 [23:33] cool [23:33] that's what bzrlib does, and I think it's drifted into a lot of lp.code(hosting)? [23:33] +1 [23:33] yeah, consensus seemed to be behind this. where there was some disagreement was on forcing people to do that order, so we compromised :) [23:34] I'd probably enforce it [23:34] next, intellectronica outlined some of the things that the ajax swat team's been discussing about js branches [23:34] thumper: i would too, but there was no consensus, so "strongly prefer" seemed good enough [23:35] barry: strongly prefer but individual reviewers may enforce L) [23:35] :) [23:35] more on this, or move on? [23:35] move on [23:35] wait [23:35] are there any actual _points_ from the js branches? [23:36] or was it just a discussion? [23:36] jml: yes. if we're moving on, i'll cut&paste intellectronica's points :) [23:36] go go [23:36] code that is OO and modularized is much easier to work with than code that is very imperative and/or relies on closure too heavily [23:37] unit tests (aka YUI tests) are a great way to test our code. in many cases much better than integration tests [23:37] working on UI features it is often easier to first concentrate on completing a working UI than on code quality. that's fine, but it means that we should refactor the code immediately after it passes UI review. that can happen either before code review, or if not, the reviewer should encourage doing that (either as part of this branch or in a followup branch). [23:37] finally, since javascript code is so much more verbose, we propose a much higher limit, in order to encourage developers to not skip these important steps. 1600 lines [23:37] look in .../javascript/*/tests for unittest examples; only soyuz is in our tree atm. or look in lazr-js. [23:37] EOT [23:38] I would add "largely functional" to "OO and modularized" [23:38] question [23:38] but that's just me. [23:38] where is lp.code javascript supposed to go? [23:38] thumper: i don't know. this morning was the first i heard of that stuff [23:39] the existing stuff is in l/c/l/javascript/* [23:39] so... old tree [23:39] hmm.. [23:39] who is going to move this? [23:39] but i know very little about it (first looked at it about 10 minutes ago) [23:39] thumper: another good question. i don't even know how to /run/ it [23:40] it seems we have a meeting of the js-resistant! [23:40] true [23:41] maybe when i send the minutes of the meetings, we can start a discussion of these on the ml [23:41] i really can't give you much more than what i just pasted ;) [23:41] :) [23:41] thumper has a topic when we're ready to move on [23:41] It'd be nice if we could have the javascript for the apps in the right part of the tree [23:41] I'm ready to move on :) [23:42] move on [23:42] my topic [23:42] thumper: it's all yours [23:42] this week the code team removed all lp.code imports from canonical.launchpad.database.__init__.py and canonical.launchpad.interfaces.__init__.py [23:42] the team leads agreed to get this done for their projects [23:43] we should make sure there are no imports from these modules in reviews [23:43] to make their job easier [23:43] I personally changed many IPerson and IProduct imports to lp.registry [23:44] thumper: awesome. new is better :) [23:44] me too, actually. [23:44] that's it [23:44] +1 [23:44] +1 [23:44] +1 [23:44] thanks thumper [23:44] sold! [23:44] anything else guys? [23:44] barry, one more thing [23:45] jml: you're up [23:45] barry, I'd be kind of interested in knowing how many reviews people do per day. [23:45] jml: we have real metrics [23:45] barry, because I'm thinking that the small size of asiapac lends itself (perversely) to everyone doing more reviews. [23:45] jml: in LP [23:46] thumper, extract them for me :) [23:46] yes, that would be cool to know! thumper can you get a losa to run a report? [23:46] jml: some LP lib magic should be able to do this [23:46] barry: I don't need no stinking losas :) [23:46] barry, very low priority, I guess. I've just been noticing how often I get interrupted, and wonder if Almighty Process can make that happen less :) [23:46] or that :) [23:46] * mthaddon stares at thumper [23:47] mthaddon: ... to run a script for me :) [23:47] that's it. [23:47] * thumper can do it himself :) [23:47] jml: how does ocr work for you guys? do you do it, or do you work on demand? [23:47] * mthaddon is actually happy (apart from the stinking part) [23:47] barry, we do work on demand [23:47] AND [23:47] when we are ocrs, we do a lot of reviews [23:47] barry: I don't do OCR, but do quite a few on demand [23:48] often more from the americas on OCR days. [23:48] jml: when you ocr do you just pop branches off activereviews or wait for devs to come to you. and if the latter, do you get many non-asiapac branches? [23:48] i don't really do ocr, to be honest [23:48] barry, I pop off to a limit of 3. :) [23:48] barry, or if they are small. [23:48] mwhudson, thumper gotcha [23:49] generally whenever anyone says "can you do a review" i do it [23:49] barry, I do get people who come to me, but it varies from week to week. [23:49] (ocr day or not) [23:49] jml: makes sense. actually, interestingly enough, your BOD overlaps with our EOD, so you could do some on demand america reviews occasionally [23:49] barry, I sometimes do. [23:49] that's great, actually [23:50] (also, it's different in summer) [23:50] well, thumper if you run that script, do let us know. it would be interesting [23:50] jml: who's summer? [23:50] kk [23:50] anything else guys? [23:51] that's it from me. [23:51] * thumper is done [23:51] all from me [23:51] thanks barry [23:51] thanks guys! [23:51] #endmeeting [23:51] Meeting finished at 17:51. [23:51] * barry -> dinner [23:51] thanks barry