= ReviewerMeeting20090318 = == summary == * when reviewing tal, please verify anything that uses `structure` works in the containing elements. * tests may be in doctest or unittest style under the discretion of the developer and/or team == log == {{{ Mar 11 10:00:03 #startmeeting Mar 11 10:00:03 Meeting started at 09:00. The chair is barry. Mar 11 10:00:03 Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] Mar 11 10:00:18 hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? Mar 11 10:00:19 me Mar 11 10:00:20 me Mar 11 10:00:24 ma Mar 11 10:00:26 ma Mar 11 10:00:32 me Mar 11 10:00:32 :-) Mar 11 10:00:33 where is my e Mar 11 10:00:36 me Mar 11 10:00:40 moi Mar 11 10:00:59 me Mar 11 10:01:09 noodles775, you need to step one country to the right Mar 11 10:01:23 mars: ah, wrong one... :/ Mar 11 10:01:33 bac: ping Mar 11 10:01:43 argh Mar 11 10:01:44 me Mar 11 10:01:49 bigjools, cprov, danilo_ ping Mar 11 10:02:01 intellectronica: ping Mar 11 10:02:11 me Mar 11 10:02:12 rockstar: ping Mar 11 10:02:14 * flacoste (n=francis@canonical/launchpad/flacoste) has joined #launchpad-meeting Mar 11 10:02:19 salgado: ping Mar 11 10:02:45 today's a light day, so let's start off with the fun stuff Mar 11 10:02:45 me! Mar 11 10:02:45 me Mar 11 10:02:56 me Mar 11 10:03:05 [TOPIC] agenda Mar 11 10:03:06 New Topic: agenda Mar 11 10:03:13 * Roll call Mar 11 10:03:13 * Peanut gallery (anything not on the agenda) Mar 11 10:03:13 * Mentoring update Mar 11 10:03:13 * Action items Mar 11 10:03:23 [TOPIC] * Peanut gallery (anything not on the agenda) Mar 11 10:03:24 New Topic: * Peanut gallery (anything not on the agenda) Mar 11 10:03:39 me Mar 11 10:03:41 does anybody have any reviewy type stuff they want to bring up that's not on the agenda? Mar 11 10:03:41 * danilo_ is now known as danilos Mar 11 10:03:48 me Mar 11 10:04:02 sinzui: the floor is yours Mar 11 10:04:08 bac discovered that there were a few places in our templates where we were making insane links. Mar 11 10:04:08 Bad Mar 11 10:04:08 Mar 11 10:04:08 becomes Mar 11 10:04:08 text Mar 11 10:04:09 Good Mar 11 10:04:11 Mar 11 10:04:13 becomes Mar 11 10:04:15 text Mar 11 10:04:39 When reviewing tal, please verify anything that uses `structure` works in the containing elements. Mar 11 10:05:20 that's wacky Mar 11 10:05:45 sinzui, would html-tidy catch that? Mar 11 10:05:58 mars yes Mar 11 10:06:13 after rendering, yes, but not before rendering (in our templates) Mar 11 10:06:25 yep, of course Mar 11 10:06:26 mars: any HTML validator can see that is not permitted Mar 11 10:06:48 not that we have any good or automated way to run said validator... Mar 11 10:06:50 i have a branch that attempt to ferret them all out. even our 'featured projects' listing on the front page had the problem. Mar 11 10:07:47 good catch sinzui and bac Mar 11 10:08:11 bac: by visual inspection or through some tool? Mar 11 10:08:24 well, when sinzui says "bac discovered" he means "bac repeated the problem flagrantly" such that sinzui discovered it. Mar 11 10:08:37 what if we made the doctest runner do HTML validation on page open? Mar 11 10:08:40 :-) Mar 11 10:08:44 barry: just grep and examing them all Mar 11 10:09:17 barry 'content="structure .*link' catches most of them Mar 11 10:09:40 mars: interesting idea, but i'm affraid it would slow things down alot Mar 11 10:09:43 of course we can write tal on multiple lines Mar 11 10:10:11 [blue sky] we could have a nightly buildbot for what mars suggests Mar 11 10:10:20 barry, we could bind it to 'make check' Mar 11 10:10:28 mars: too slow, as is updating find_main_content Mar 11 10:10:35 barry, since we'll have to do so for windmill tests anyway Mar 11 10:10:48 which are even slower Mar 11 10:11:04 Our link checker could take on the extra work. or... Mar 11 10:11:20 mars: yeah Mar 11 10:11:24 sinzui, yes, that's another good hook point Mar 11 10:11:25 >>> view = create_initialized_view(person, '+index', principle=person) Mar 11 10:11:25 >>> is_valid_html(view.render()) Mar 11 10:11:25 True Mar 11 10:11:36 sinzui, but the link-checker isn't under developer control Mar 11 10:11:46 is it? Mar 11 10:12:01 gary_poster: +1 -- a periodic scan should be enough to trap infrequent offenders Mar 11 10:12:01 at least I could do ./test.py --validate-markup or something Mar 11 10:13:08 * Ursinha has quit (Read error: 110 (Connection timed out)) Mar 11 10:13:12 sinzui: i think i like that approach best. cons: it's up to the dev to add the check, pros: you can decide to check only when you think it's necessary Mar 11 10:14:06 does someone want to take this on as an action item? Mar 11 10:14:30 i think the buldbot story is better Mar 11 10:14:31 here Mar 11 10:14:33 cons: it slows down our tests Mar 11 10:14:38 doesn't slow down the test suite Mar 11 10:14:46 it's not a big problem Mar 11 10:15:00 i mean this has been broken for long and didn't cause any problems Mar 11 10:15:10 invalid markup is ugly, but not a show-stoppers Mar 11 10:15:20 thanks to the way browsers are implemented Mar 11 10:15:27 we don't need to solve it here, but i'd like someone to own it and offer some options Mar 11 10:15:47 flacoste, does our build process have provisions or the concept of warnings? Mar 11 10:15:47 it sounds like a foundations issue Mar 11 10:15:54 but it would be a very low priority to me Mar 11 10:16:01 so no point putting it down as an actino Mar 11 10:16:09 flacoste: okay Mar 11 10:16:57 thanks. any other topics not on the agenda? Mar 11 10:18:02 [TOPIC] mentoring update Mar 11 10:18:03 New Topic: mentoring update Mar 11 10:18:11 any mentor or mentat updates today? Mar 11 10:18:47 me Mar 11 10:18:53 * ursula_ (n=ursula@canonical/launchpad/ursinha) has joined #launchpad-meeting Mar 11 10:19:10 * ursula_ is now known as ghost Mar 11 10:19:18 * ghost is now known as Ursinha Mar 11 10:19:30 rockstar: you don't have a mentoring update, right? Mar 11 10:19:45 No, I'm just screwed by DST again. Mar 11 10:19:56 [TOPIC] action items Mar 11 10:19:57 New Topic: action items Mar 11 10:20:05 * gary to add `getStore()` as an alias for `_get_store()` Mar 11 10:20:41 gary_poster: ^^ Mar 11 10:21:21 barry: i still suck at mine, but do keep it open, i plan to suck less Mar 11 10:21:31 see, i'm pro-active :-) Mar 11 10:21:38 flacoste: that's a start! :) Mar 11 10:21:53 * gary will check to see if there's a bug open for adding a hook to `bzr send`, and submit one if there isn't Mar 11 10:21:57 gary_poster: ^^ Mar 11 10:22:23 * bigjools to take crack at helper functions for backpatching schemas to avoid circular imports Mar 11 10:22:36 * barry to add `field_id` to coding guideline Mar 11 10:22:36 * barry to update guidelines to never call `_foo()` methods from outside a class Mar 11 10:22:36 * barry to add `pretty()` functions to reviewers docs Mar 11 10:22:47 i actually do NOT suck today! i did all three of these :) Mar 11 10:23:01 goddamn it and the time changes Mar 11 10:23:34 * bigjools sucks at helper functions AND timekeeping Mar 11 10:24:10 no worries Mar 11 10:24:23 anyway, that's all i have today. anything else going on? Mar 11 10:24:52 5 Mar 11 10:25:05 I have something Mar 11 10:25:17 rockstar: go ahead Mar 11 10:25:26 It looks like sinzui already quoted to code above... Mar 11 10:25:50 I was reviewing a branch of sinzui's where he was using a doctest to test a view's API. Mar 11 10:26:33 Now, I understand that our team has an unusual fetish for doctests, but I think that if we're doing something where we can use unittests, we should use them. Mar 11 10:27:30 hmm Mar 11 10:27:33 * mars sees the testing debate from the ML creeping in... Mar 11 10:27:36 that's a controversial topic Mar 11 10:27:43 rockstar: what are the arguments? Mar 11 10:27:53 i might make the counter argument :) Mar 11 10:28:04 yeah Mar 11 10:28:05 Shall we wall read the 2008 June, July debate. Mar 11 10:28:14 doctests are good for telling a user story, but if we're just testing the methods of a view, why not use what unittests for what they were made for? Mar 11 10:28:19 I believe thumper wanted a paintball battle to decide the matter Mar 11 10:28:26 lol Mar 11 10:28:38 sinzui: M-x paintball Mar 11 10:28:40 rockstar: we have usually refrained from enforcing a policy there Mar 11 10:28:51 let developers choose which technology they prefer here Mar 11 10:28:55 * gary_poster_ (n=gary_pos@pool-72-86-46-134.clppva.fios.verizon.net) has joined #launchpad-meeting Mar 11 10:29:13 flacoste: right. i think that still works as a policy Mar 11 10:29:20 flacoste, and I think that leads to a mess. Mar 11 10:29:28 i think that unless we lose coverage because of that, we should allow teams to choose what format they prefer Mar 11 10:29:40 for individuals to choose is already a bit extreme Mar 11 10:29:45 If I change a view, I can't easily discern where the hell the test is that I need to edit. Mar 11 10:30:03 Is it it doc? Is it in browser/tests? Mar 11 10:30:13 I'm agnostic on the issue. Since all the registry view tests are in doctest, I wan to add to them instead of storing two kinds of tests in two areas Mar 11 10:30:21 rockstar: conversely without a doctest if i'm trying to learn how a view works, it's much more difficult to discern that from its docstrings and unittest Mar 11 10:30:43 barry, why can't you look at the unittest? You'll be doing essentially the same thing. Mar 11 10:31:11 rockstar: doctests do not normally explain WHY the view does soemthing Mar 11 10:31:32 rockstar: unittests lack the narrative that sets the context of the api Mar 11 10:31:38 right Mar 11 10:31:40 rockstar finding the tests is different than the format Mar 11 10:31:41 unittests are superior ways of testing, I'm pretty tired of having a doctest halt when one of its tests fails Mar 11 10:31:41 the "ties it all together" bits Mar 11 10:31:53 it should be browser/tests Mar 11 10:31:57 whatever the format Mar 11 10:32:01 unless it's an API description Mar 11 10:32:04 and even then Mar 11 10:32:20 i'm starting the Launchpad-tree-apocalypse today Mar 11 10:32:29 /o\ Mar 11 10:32:38 * sinzui hand flacoste the fuel and a torch Mar 11 10:32:40 I think mixing and matching tests is going to bite us one of these days. Mar 11 10:32:41 that will makes things clearer for finding stuff and tests Mar 11 10:32:43 for answers Mar 11 10:32:52 and registry will they their apocalyspe next week Mar 11 10:33:10 gary_poster, zope has a lot of experience in that area Mar 11 10:33:16 * sinzui is bringing marshmallows. Mar 11 10:33:17 anything to share here? Mar 11 10:33:45 Anytime you say "In this folder, you might find unittests or doctests" it makes me think that one or the other is being misused. Mar 11 10:34:07 flacoste: sorry, having connectivity issues and buildbot issues simultaneously Mar 11 10:34:07 rockstar: personally, i'd like to see doctests in a 'docs' directory Mar 11 10:34:40 sanitizing our testing infrastructure is definitely on the list of monsters to pitchfork Mar 11 10:35:15 barry, personally, I'd like to see the doctests use testbrowser to tell a user story, or unittests to test a unit. Mar 11 10:35:59 barry: well, if it's docs, it belongs in a docs directory Mar 11 10:36:09 but many doctest are not docs Mar 11 10:36:11 rockstar: what about documentation for internal components and subsystems? Mar 11 10:36:19 rockstar: do you never use the doctests to learn about functionality? i often do, and i think that if we don't have doctests, we won't have documentation at all Mar 11 10:36:20 flacoste: yes, that's a problem :) Mar 11 10:36:21 docs is about API description Mar 11 10:36:24 well, it's not Mar 11 10:36:25 flacoste, isn't that the benefit of doctests? They are docs AND they are tests? Mar 11 10:36:25 rockstar: model/components need to be doctests because their are the develop documentation. Mar 11 10:36:36 well Mar 11 10:36:43 i don't think a view is an API Mar 11 10:36:50 interesting to document Mar 11 10:36:53 most of them aren't Mar 11 10:36:55 a few might be Mar 11 10:36:59 but they are the exception Mar 11 10:37:14 flacoste, a view has an API. Mar 11 10:37:24 well, not really Mar 11 10:37:31 the only API is __call__ Mar 11 10:37:35 which renders the view Mar 11 10:37:37 that's it Mar 11 10:37:38 flacoste, and the properties Mar 11 10:37:42 no Mar 11 10:37:48 intellectronica, I look at unittests all the time to learn about functionality. Mar 11 10:37:48 well, maybe Mar 11 10:37:56 rockstar: i'm thinking about the multistep view i just refactored. i think documentation for it is essential so that others can understand how to reuse it. Mar 11 10:38:00 i would argue that the properties are internal implementation details Mar 11 10:38:01 My thoughts on this: (1) doctest for unittest purpose is emacs vs. vi IMO/IME. I think that using doctest for unittest purposes works well. Other people disagree. I tend to find that people think about these things differently. I can acknowledge the points people make (like bigjools, IRT doctests failing at the beginning) but I prefer doctests. Mar 11 10:38:17 (2) I would much prefer it if our tests were closer to the pertinent code Mar 11 10:38:35 i agree with both points Mar 11 10:38:40 that appears to be (part of) the topic I lost while I was dealing with various issues Mar 11 10:38:41 gary_poster, +1 on (2) Mar 11 10:38:41 and my apocalyspe branch will improve 2 Mar 11 10:39:03 flacoste, rockstar, maybe reorganize the tree, and wait and see? Mar 11 10:39:05 * flacoste can't spell apocalypse Mar 11 10:39:21 you can run more experiments, say with the view tests, after Mar 11 10:39:22 flacoste: maybe that should be apocalisp? Mar 11 10:39:28 lol Mar 11 10:39:29 :-) Mar 11 10:39:40 a smaller sandbox will make experimenting on a per-team basis easier Mar 11 10:39:54 mars, I don't think that's the answer. Mar 11 10:39:57 mars: I think teams have already made their decision Mar 11 10:40:00 I see doctests primarily as documentation that carry examples, it's easy to abuse them to be full-blown tests Mar 11 10:40:05 mars: yes, and it will make things much more discoverable and comprehensible Mar 11 10:40:28 The per-team thing just makes things messier. That's why it's good to have cross-team-reviews Mar 11 10:40:34 I think that teams that will not release their code are using unittests. Mar 11 10:40:50 sinzui, :( Mar 11 10:40:51 * gary_poster has quit (Read error: 113 (No route to host)) Mar 11 10:41:15 * gary_poster_ is now known as gary_poster Mar 11 10:41:16 rockstar: well, not really per-team. what i meant, at least, is that the division should be thematic, not personal Mar 11 10:41:20 i think it's just too controversial to mandate at this point. i agree with gary that it feels like mandating vi or emacs Mar 11 10:41:40 intellectronica, depending on the "theme" I agree with you. Mar 11 10:41:41 perhaps emacs users could use doctests... Mar 11 10:41:45 :-) Mar 11 10:42:00 i personally think that both doctests and unittests have their place, both have positives... and negatives Mar 11 10:42:07 barry, I don't see why. I think they serve two different purposes. It's like mandating motor oil v. beer. Mar 11 10:42:21 rockstar, yuck Mar 11 10:42:38 rockstar: right, but...other people disagree with you :-) Mar 11 10:42:52 that's the point Mar 11 10:42:53 rockstar: is that: unittests are greasy and icky but good for your motor, while doctests are fun, festive and easy to consume? Mar 11 10:42:57 I think the crucial point that rockstar is making is that to test a class of problems, we should use one kind of test so that we know where and how to test it. Mar 11 10:43:14 barry: lol Mar 11 10:43:16 sinzui, yes! Thank you! Mar 11 10:43:23 My concern is that if I switch to unittest, I then have lots of tests in doctest Mar 11 10:43:40 insert old in the above sentece somewhere Mar 11 10:43:57 do we agree that doctests are not tests, they're documentation? Mar 11 10:44:01 no Mar 11 10:44:05 bigjools, no Mar 11 10:44:10 not as we use them atm. Mar 11 10:44:12 no Mar 11 10:44:15 no Mar 11 10:44:30 doctests are tests Mar 11 10:44:31 because I remember a mail thread from 2 years ago Mar 11 10:44:34 it's not either or. doctests are documentation /first/ but they are tests too Mar 11 10:44:35 I think doctests are great at telling a story about a user experience, and asserting that the story actually works with code. Mar 11 10:44:40 that said they are not tests Mar 11 10:44:52 barry: i would even argue that some doctests are not even doc first and that's fine too Mar 11 10:44:58 s/user experience/developer experience/ Mar 11 10:44:58 and that writing words like "should" are not acceptable Mar 11 10:45:06 doctests CAN be tests. unit tests CAN'T be documentation Mar 11 10:45:19 * noodles775 suddenly realizes why the email discussion the other day stopped so quickly... Mar 11 10:45:24 barry: right, testable documentation Mar 11 10:45:42 * adeuring (n=abel@p4FC54478.dip.t-dialin.net) has joined #launchpad-meeting Mar 11 10:45:43 intellectronica: i agree with that statement (flacoste's too) Mar 11 10:45:47 barry, why would there need to be a story about developer experience. "The developer can instantiate this class, and then you have these functions..." Mar 11 10:45:56 The problem comes with - to pick an example at random - things like doc/externalbugtracker-comment-pushing.txt Mar 11 10:46:09 In which we create a load of mock objects in the doctest in order to be able to run the tests. Mar 11 10:46:14 That seems unit-testy. Mar 11 10:46:19 bigjools: s/should/can|expected|required/ Mar 11 10:46:21 rockstar: ever tried to understand how a class is supposed to be used by pydoctor api reference? it sucks. unittests are the same Mar 11 10:46:50 sinzui: "will" Mar 11 10:46:52 gmb: actually, i think that's a good example to the contrary. as someone who doesn't know that system as good as you, i really benefit from having documentation whenever i need to work on it Mar 11 10:46:59 rockstar: otoh, once you understand how a class is to be used, api reference is great Mar 11 10:47:01 bigjools: +1 Mar 11 10:47:19 doctests are statements of intent Mar 11 10:47:23 I would like to withdraw my request to use unittest to test units. It seems that it's not getting anywhere. Mar 11 10:47:28 The meeting is getting close the the end. I just read unit-testies Mar 11 10:47:29 intellectronica: Hmm, okay. Then I'd argue that the mock objects should at least be factored out into ftests/somethingorother for cleanliness. Mar 11 10:47:38 sinzui: lol Mar 11 10:47:43 sinzui: on that note... Mar 11 10:47:57 fwiw, footnotes can help with this Mar 11 10:48:11 rockstar: gracious of you. as a reviewer, you still have the option of asking that something be unit tested if you think it makes more sense Mar 11 10:48:12 i propose we close the meeting for today and schedule a mud wrasslin' contest to settle this once and for all Mar 11 10:48:18 at all hands Mar 11 10:48:30 and fwiw, this example has been pointed to in the past by others as a reasonable unit test/doc test combo: http://svn.zope.org/zc.queue/trunk/src/zc/queue/queue.txt?view=auto Mar 11 10:48:48 intellectronica, as a reviewer, I don't think that makes a lot of sense if there isn't really a standard. :/ Mar 11 10:48:56 (I wrote it, so this the importance of "by others") Mar 11 10:49:23 I will propose footnotes at another mtg :-) Mar 11 10:49:48 okay, thanks everyone for a spirited debate! Mar 11 10:50:01 apologies for going over. i'm sure we'll continue this at another time Mar 11 10:50:06 #endmeeting }}}