launchpad development
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 <barry> #startmeeting Mar 11 10:00:03 <MootBot> Meeting started at 09:00. The chair is barry. Mar 11 10:00:03 <MootBot> Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] Mar 11 10:00:18 <barry> hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? Mar 11 10:00:19 <gmb> me Mar 11 10:00:20 <gary_poster> me Mar 11 10:00:24 <sinzui> ma Mar 11 10:00:26 <sinzui> ma Mar 11 10:00:32 <mars> me Mar 11 10:00:32 <gary_poster> :-) Mar 11 10:00:33 <sinzui> where is my e Mar 11 10:00:36 <sinzui> me Mar 11 10:00:40 <noodles775> moi Mar 11 10:00:59 <allenap> me Mar 11 10:01:09 <mars> noodles775, you need to step one country to the right Mar 11 10:01:23 <noodles775> mars: ah, wrong one... :/ Mar 11 10:01:33 <barry> bac: ping Mar 11 10:01:43 <bac> argh Mar 11 10:01:44 <bac> me Mar 11 10:01:49 <barry> bigjools, cprov, danilo_ ping Mar 11 10:02:01 <barry> intellectronica: ping Mar 11 10:02:11 <intellectronica> me Mar 11 10:02:12 <barry> rockstar: ping Mar 11 10:02:14 * flacoste (n=francis@canonical/launchpad/flacoste) has joined #launchpad-meeting Mar 11 10:02:19 <barry> salgado: ping Mar 11 10:02:45 <barry> today's a light day, so let's start off with the fun stuff Mar 11 10:02:45 <salgado> me! Mar 11 10:02:45 <cprov> me Mar 11 10:02:56 <flacoste> me Mar 11 10:03:05 <barry> [TOPIC] agenda Mar 11 10:03:06 <MootBot> New Topic: agenda Mar 11 10:03:13 <barry> * Roll call Mar 11 10:03:13 <barry> * Peanut gallery (anything not on the agenda) Mar 11 10:03:13 <barry> * Mentoring update Mar 11 10:03:13 <barry> * Action items Mar 11 10:03:23 <barry> [TOPIC] * Peanut gallery (anything not on the agenda) Mar 11 10:03:24 <MootBot> New Topic: * Peanut gallery (anything not on the agenda) Mar 11 10:03:39 <danilo_> me Mar 11 10:03:41 <barry> 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 <sinzui> me Mar 11 10:04:02 <barry> sinzui: the floor is yours Mar 11 10:04:08 <sinzui> bac discovered that there were a few places in our templates where we were making insane links. Mar 11 10:04:08 <sinzui> Bad Mar 11 10:04:08 <sinzui> <a tal:content="structure person/fmt:link" /> Mar 11 10:04:08 <sinzui> becomes Mar 11 10:04:08 <sinzui> <a><a href="...">text</a></a> Mar 11 10:04:09 <sinzui> Good Mar 11 10:04:11 <sinzui> <a tal:replace="structure person/fmt:link" /> Mar 11 10:04:13 <sinzui> becomes Mar 11 10:04:15 <sinzui> <a href="...">text</a> Mar 11 10:04:39 <sinzui> When reviewing tal, please verify anything that uses `structure` works in the containing elements. Mar 11 10:05:20 <barry> that's wacky Mar 11 10:05:45 <mars> sinzui, would html-tidy catch that? Mar 11 10:05:58 <sinzui> mars yes Mar 11 10:06:13 <gary_poster> after rendering, yes, but not before rendering (in our templates) Mar 11 10:06:25 <mars> yep, of course Mar 11 10:06:26 <sinzui> mars: any HTML validator can see that is not permitted Mar 11 10:06:48 <mars> not that we have any good or automated way to run said validator... Mar 11 10:06:50 <bac> 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 <flacoste> good catch sinzui and bac Mar 11 10:08:11 <barry> bac: by visual inspection or through some tool? Mar 11 10:08:24 <bac> well, when sinzui says "bac discovered" he means "bac repeated the problem flagrantly" such that sinzui discovered it. Mar 11 10:08:37 <mars> what if we made the doctest runner do HTML validation on page open? Mar 11 10:08:40 <gary_poster> :-) Mar 11 10:08:44 <bac> barry: just grep and examing them all Mar 11 10:09:17 <sinzui> barry 'content="structure .*link' catches most of them Mar 11 10:09:40 <barry> mars: interesting idea, but i'm affraid it would slow things down alot Mar 11 10:09:43 <sinzui> of course we can write tal on multiple lines Mar 11 10:10:11 <gary_poster> [blue sky] we could have a nightly buildbot for what mars suggests Mar 11 10:10:20 <mars> barry, we could bind it to 'make check' Mar 11 10:10:28 <sinzui> mars: too slow, as is updating find_main_content Mar 11 10:10:35 <mars> barry, since we'll have to do so for windmill tests anyway Mar 11 10:10:48 <mars> which are even slower Mar 11 10:11:04 <sinzui> Our link checker could take on the extra work. or... Mar 11 10:11:20 <barry> mars: yeah Mar 11 10:11:24 <mars> sinzui, yes, that's another good hook point Mar 11 10:11:25 <sinzui> >>> view = create_initialized_view(person, '+index', principle=person) Mar 11 10:11:25 <sinzui> >>> is_valid_html(view.render()) Mar 11 10:11:25 <sinzui> True Mar 11 10:11:36 <mars> sinzui, but the link-checker isn't under developer control Mar 11 10:11:46 <mars> is it? Mar 11 10:12:01 <bac> gary_poster: +1 -- a periodic scan should be enough to trap infrequent offenders Mar 11 10:12:01 <mars> 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 <barry> 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 <barry> does someone want to take this on as an action item? Mar 11 10:14:30 <flacoste> i think the buldbot story is better Mar 11 10:14:31 <flacoste> here Mar 11 10:14:33 <bac> cons: it slows down our tests Mar 11 10:14:38 <flacoste> doesn't slow down the test suite Mar 11 10:14:46 <flacoste> it's not a big problem Mar 11 10:15:00 <flacoste> i mean this has been broken for long and didn't cause any problems Mar 11 10:15:10 <flacoste> invalid markup is ugly, but not a show-stoppers Mar 11 10:15:20 <flacoste> thanks to the way browsers are implemented Mar 11 10:15:27 <barry> 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 <mars> flacoste, does our build process have provisions or the concept of warnings? Mar 11 10:15:47 <flacoste> it sounds like a foundations issue Mar 11 10:15:54 <flacoste> but it would be a very low priority to me Mar 11 10:16:01 <flacoste> so no point putting it down as an actino Mar 11 10:16:09 <barry> flacoste: okay Mar 11 10:16:57 <barry> thanks. any other topics not on the agenda? Mar 11 10:18:02 <barry> [TOPIC] mentoring update Mar 11 10:18:03 <MootBot> New Topic: mentoring update Mar 11 10:18:11 <barry> any mentor or mentat updates today? Mar 11 10:18:47 <rockstar> 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 <barry> rockstar: you don't have a mentoring update, right? Mar 11 10:19:45 <rockstar> No, I'm just screwed by DST again. Mar 11 10:19:56 <barry> [TOPIC] action items Mar 11 10:19:57 <MootBot> New Topic: action items Mar 11 10:20:05 <barry> * gary to add `getStore()` as an alias for `_get_store()` Mar 11 10:20:41 <barry> gary_poster: ^^ Mar 11 10:21:21 <flacoste> barry: i still suck at mine, but do keep it open, i plan to suck less Mar 11 10:21:31 <flacoste> see, i'm pro-active :-) Mar 11 10:21:38 <barry> flacoste: that's a start! :) Mar 11 10:21:53 <barry> * 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 <barry> gary_poster: ^^ Mar 11 10:22:23 <barry> * bigjools to take crack at helper functions for backpatching schemas to avoid circular imports Mar 11 10:22:36 <barry> * barry to add `field_id` to coding guideline Mar 11 10:22:36 <barry> * barry to update guidelines to never call `_foo()` methods from outside a class Mar 11 10:22:36 <barry> * barry to add `pretty()` functions to reviewers docs Mar 11 10:22:47 <barry> i actually do NOT suck today! i did all three of these :) Mar 11 10:23:01 <bigjools> goddamn it and the time changes Mar 11 10:23:34 * bigjools sucks at helper functions AND timekeeping Mar 11 10:24:10 <barry> no worries Mar 11 10:24:23 <barry> anyway, that's all i have today. anything else going on? Mar 11 10:24:52 <barry> 5 Mar 11 10:25:05 <rockstar> I have something Mar 11 10:25:17 <barry> rockstar: go ahead Mar 11 10:25:26 <rockstar> It looks like sinzui already quoted to code above... Mar 11 10:25:50 <rockstar> 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 <rockstar> 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 <flacoste> hmm Mar 11 10:27:33 * mars sees the testing debate from the ML creeping in... Mar 11 10:27:36 <flacoste> that's a controversial topic Mar 11 10:27:43 <flacoste> rockstar: what are the arguments? Mar 11 10:27:53 <barry> i might make the counter argument :) Mar 11 10:28:04 <flacoste> yeah Mar 11 10:28:05 <sinzui> Shall we wall read the 2008 June, July debate. Mar 11 10:28:14 <rockstar> 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 <sinzui> I believe thumper wanted a paintball battle to decide the matter Mar 11 10:28:26 <flacoste> lol Mar 11 10:28:38 <barry> sinzui: M-x paintball Mar 11 10:28:40 <flacoste> rockstar: we have usually refrained from enforcing a policy there Mar 11 10:28:51 <flacoste> 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 <barry> flacoste: right. i think that still works as a policy Mar 11 10:29:20 <rockstar> flacoste, and I think that leads to a mess. Mar 11 10:29:28 <intellectronica> 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 <intellectronica> for individuals to choose is already a bit extreme Mar 11 10:29:45 <rockstar> 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 <rockstar> Is it it doc? Is it in browser/tests? Mar 11 10:30:13 <sinzui> 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 <barry> 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 <rockstar> barry, why can't you look at the unittest? You'll be doing essentially the same thing. Mar 11 10:31:11 <sinzui> rockstar: doctests do not normally explain WHY the view does soemthing Mar 11 10:31:32 <barry> rockstar: unittests lack the narrative that sets the context of the api Mar 11 10:31:38 <mars> right Mar 11 10:31:40 <flacoste> rockstar finding the tests is different than the format Mar 11 10:31:41 <bigjools> 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 <barry> the "ties it all together" bits Mar 11 10:31:53 <flacoste> it should be browser/tests Mar 11 10:31:57 <flacoste> whatever the format Mar 11 10:32:01 <flacoste> unless it's an API description Mar 11 10:32:04 <flacoste> and even then Mar 11 10:32:20 <flacoste> i'm starting the Launchpad-tree-apocalypse today Mar 11 10:32:29 <bigjools> /o\ Mar 11 10:32:38 * sinzui hand flacoste the fuel and a torch Mar 11 10:32:40 <rockstar> I think mixing and matching tests is going to bite us one of these days. Mar 11 10:32:41 <flacoste> that will makes things clearer for finding stuff and tests Mar 11 10:32:43 <flacoste> for answers Mar 11 10:32:52 <flacoste> and registry will they their apocalyspe next week Mar 11 10:33:10 <flacoste> 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 <flacoste> anything to share here? Mar 11 10:33:45 <rockstar> 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 <gary_poster_> flacoste: sorry, having connectivity issues and buildbot issues simultaneously Mar 11 10:34:07 <barry> rockstar: personally, i'd like to see doctests in a 'docs' directory Mar 11 10:34:40 <barry> sanitizing our testing infrastructure is definitely on the list of monsters to pitchfork Mar 11 10:35:15 <rockstar> 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 <flacoste> barry: well, if it's docs, it belongs in a docs directory Mar 11 10:36:09 <flacoste> but many doctest are not docs Mar 11 10:36:11 <barry> rockstar: what about documentation for internal components and subsystems? Mar 11 10:36:19 <intellectronica> 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 <barry> flacoste: yes, that's a problem :) Mar 11 10:36:21 <flacoste> docs is about API description Mar 11 10:36:24 <flacoste> well, it's not Mar 11 10:36:25 <rockstar> flacoste, isn't that the benefit of doctests? They are docs AND they are tests? Mar 11 10:36:25 <sinzui> rockstar: model/components need to be doctests because their are the develop documentation. Mar 11 10:36:36 <flacoste> well Mar 11 10:36:43 <flacoste> i don't think a view is an API Mar 11 10:36:50 <flacoste> interesting to document Mar 11 10:36:53 <flacoste> most of them aren't Mar 11 10:36:55 <flacoste> a few might be Mar 11 10:36:59 <flacoste> but they are the exception Mar 11 10:37:14 <rockstar> flacoste, a view has an API. Mar 11 10:37:24 <flacoste> well, not really Mar 11 10:37:31 <flacoste> the only API is __call__ Mar 11 10:37:35 <flacoste> which renders the view Mar 11 10:37:37 <flacoste> that's it Mar 11 10:37:38 <mars> flacoste, and the properties Mar 11 10:37:42 <flacoste> no Mar 11 10:37:48 <rockstar> intellectronica, I look at unittests all the time to learn about functionality. Mar 11 10:37:48 <flacoste> well, maybe Mar 11 10:37:56 <barry> 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 <flacoste> i would argue that the properties are internal implementation details Mar 11 10:38:01 <gary_poster_> 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 <gary_poster_> (2) I would much prefer it if our tests were closer to the pertinent code Mar 11 10:38:35 <flacoste> i agree with both points Mar 11 10:38:40 <gary_poster_> that appears to be (part of) the topic I lost while I was dealing with various issues Mar 11 10:38:41 <rockstar> gary_poster, +1 on (2) Mar 11 10:38:41 <flacoste> and my apocalyspe branch will improve 2 Mar 11 10:39:03 <mars> 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 <mars> you can run more experiments, say with the view tests, after Mar 11 10:39:22 <barry> flacoste: maybe that should be apocalisp? Mar 11 10:39:28 <flacoste> lol Mar 11 10:39:29 <gary_poster_> :-) Mar 11 10:39:40 <mars> a smaller sandbox will make experimenting on a per-team basis easier Mar 11 10:39:54 <rockstar> mars, I don't think that's the answer. Mar 11 10:39:57 <sinzui> mars: I think teams have already made their decision Mar 11 10:40:00 <bigjools> 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 <barry> mars: yes, and it will make things much more discoverable and comprehensible Mar 11 10:40:28 <rockstar> The per-team thing just makes things messier. That's why it's good to have cross-team-reviews Mar 11 10:40:34 <sinzui> I think that teams that will not release their code are using unittests. Mar 11 10:40:50 <rockstar> 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 <intellectronica> 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 <barry> 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 <rockstar> intellectronica, depending on the "theme" I agree with you. Mar 11 10:41:41 <bac> perhaps emacs users could use doctests... Mar 11 10:41:45 <gary_poster> :-) Mar 11 10:42:00 <barry> i personally think that both doctests and unittests have their place, both have positives... and negatives Mar 11 10:42:07 <rockstar> 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 <mars> rockstar, yuck Mar 11 10:42:38 <gary_poster> rockstar: right, but...other people disagree with you :-) Mar 11 10:42:52 <gary_poster> that's the point Mar 11 10:42:53 <barry> 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 <sinzui> 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 <cprov> barry: lol Mar 11 10:43:16 <rockstar> sinzui, yes! Thank you! Mar 11 10:43:23 <sinzui> My concern is that if I switch to unittest, I then have lots of tests in doctest Mar 11 10:43:40 <sinzui> insert old in the above sentece somewhere Mar 11 10:43:57 <bigjools> do we agree that doctests are not tests, they're documentation? Mar 11 10:44:01 <gary_poster> no Mar 11 10:44:05 <rockstar> bigjools, no Mar 11 10:44:10 <gmb> not as we use them atm. Mar 11 10:44:12 <barry> no Mar 11 10:44:15 <cprov> no Mar 11 10:44:30 <flacoste> doctests are tests Mar 11 10:44:31 <bigjools> because I remember a mail thread from 2 years ago Mar 11 10:44:34 <barry> it's not either or. doctests are documentation /first/ but they are tests too Mar 11 10:44:35 <rockstar> 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 <bigjools> that said they are not tests Mar 11 10:44:52 <flacoste> barry: i would even argue that some doctests are not even doc first and that's fine too Mar 11 10:44:58 <barry> s/user experience/developer experience/ Mar 11 10:44:58 <bigjools> and that writing words like "should" are not acceptable Mar 11 10:45:06 <intellectronica> 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 <bigjools> 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 <barry> intellectronica: i agree with that statement (flacoste's too) Mar 11 10:45:47 <rockstar> 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 <gmb> The problem comes with - to pick an example at random - things like doc/externalbugtracker-comment-pushing.txt Mar 11 10:46:09 <gmb> 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 <gmb> That seems unit-testy. Mar 11 10:46:19 <sinzui> bigjools: s/should/can|expected|required/ Mar 11 10:46:21 <barry> 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 <bigjools> sinzui: "will" Mar 11 10:46:52 <intellectronica> 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 <barry> rockstar: otoh, once you understand how a class is to be used, api reference is great Mar 11 10:47:01 <sinzui> bigjools: +1 Mar 11 10:47:19 <bigjools> doctests are statements of intent Mar 11 10:47:23 <rockstar> 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 <sinzui> The meeting is getting close the the end. I just read unit-testies Mar 11 10:47:29 <gmb> 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 <bigjools> sinzui: lol Mar 11 10:47:43 <barry> sinzui: on that note... Mar 11 10:47:57 <gary_poster> fwiw, footnotes can help with this Mar 11 10:48:11 <intellectronica> 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 <barry> 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 <barry> at all hands Mar 11 10:48:30 <gary_poster> 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 <rockstar> 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 <gary_poster> (I wrote it, so this the importance of "by others") Mar 11 10:49:23 <gary_poster> I will propose footnotes at another mtg :-) Mar 11 10:49:48 <barry> okay, thanks everyone for a spirited debate! Mar 11 10:50:01 <barry> apologies for going over. i'm sure we'll continue this at another time Mar 11 10:50:06 <barry> #endmeeting
