ReviewerMeeting20090909
summary
don't use __iter__() for things that aren't obviously collections
watch out for leaking the open file descriptor returned from mkstemp()
use addCleanup()
use mkdtemp()
use os.fdopen()
- watch out for cleanup of other external resources
- don't write umask-sensitive code
- do not assume umask 022
do not assume default Ubuntu user environment
watch out for devscripts and ~/.devscripts
- if a particular umask is required, add it to the code and test for it
- watch out for similar environmental sensitivities
- [ACTION] cprov to update guidelines to clarify how code sensitive to env changes should be written
HTML4 spec requires <script> to have a close tag
do not write <script type="text/javascript" src="..."/>
do not write empty <p></p>
IWBNI someone could do some gardening of https://dev.launchpad.net/StyleGuides
- cover letters are more important to be formally filled out if you're queuing up for `+activereviews
- you can be more agile
- important thing is to communicate b/w dev and reviewer
- different workflows for asiapac and ameu
- checklists are great
logs
ameu
10:00:10 > barry: #startmeeting 10:00:12 < MootBot: Meeting started at 09:00. The chair is barry. 10:00:12 < MootBot: Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] 10:00:24 > barry: hello everybody and welcome to this week's ameu reviewer's meeting. who's here today? 10:00:35 < noodles775: me 10:00:38 < henninge: me 10:00:51 < bac: me 10:00:54 < danilos: me 10:01:11 > barry: gmb sends his apologies 10:01:19 < bigjools: me 10:01:23 < adeuring: me 10:02:00 < flacoste: me 10:02:07 < henninge: jtv has better things^W^W^W cannot be here either ... 10:02:12 < henninge: ;) 10:02:15 > barry: henninge: thanks :) 10:02:31 > barry: flacoste, gary_poster, salgado, cprov, allenap, mars, sinzui ping 10:02:36 < salgado: me 10:02:41 < cprov: me 10:02:41 < sinzui: me 10:02:41 < flacoste: re-me 10:02:41 < allenap: me 10:02:43 < gary_poster: me 10:02:48 > barry: flacoste: oops! 10:02:49 -!- deryck:03:03 > barry: [TOPIC] agenda 10:03:04 < MootBot: New Topic: agenda 10:03:07 < flacoste: barry: that's ok, you are used to pinging me 10:03:19 > barry: * Roll call 10:03:19 > barry: * Action items 10:03:19 > barry: * UI review call update 10:03:19 > barry: * `__iter__()` in model objects? [cprov, barry] 10:03:22 > barry: * mkstemp()/open() combination leaks file-descriptors [cprov] 10:03:25 > barry: * Guidelines changes for tests checking systems pre-requisites (specific umasks, app configurations) [maxb, cprov] 10:03:30 > barry: * Invalid markup; The <script> tag requires a close tag (Do not: <script type="text/javascript" src="..."/>) [Curtis] 10:03:33 > barry: * 4-space indents for CSS styles [barry] 10:03:35 > barry: * Peanut gallery (anything not on the agenda) 10:03:38 > barry: 10:03:41 > barry: flacoste: :) 10:03:44 > barry: [TOPIC] action items 10:03:45 < MootBot: New Topic: action items 10:03:49 > barry: * gary_poster and barry will transfer review guidelines from the old wiki and old old wiki to the new wiki 10:03:54 > barry: we suck 10:04:01 < gary_poster: yup, joint suckage 10:04:22 > barry: gary_poster: that sounds icky :) 10:04:40 < gary_poster: heh, yeah, I thought that after I read it too 10:04:49 > barry: [TOPIC] * UI review call update 10:04:50 < MootBot: New Topic: * UI review call update 10:05:02 > barry: gary_poster: let's get together later today to plan that out 10:05:10 < gary_poster: +1 10:05:26 > barry: i was not on the ui review call because of the us holiday. was anybody here on that call, and was anything interesting discussed? 10:05:54 < noodles775: barry: it was canceled as there was only intellectronica jtv and myself. 10:06:16 > barry: noodles775: cool, thanks 10:06:34 > barry: [TOPIC] * `__iter__()` in model objects? [cprov, barry] 10:06:36 < MootBot: New Topic: * `__iter__()` in model objects? [cprov, barry] 10:06:43 > barry: cprov: do you remember what this one was about? 10:06:44 -!- intellectronica:06:48 < intellectronica: me too 10:07:43 < cprov: barry: well, it's about the how confusing it is to read code that iterate over a distroseries and get distroarchseries ;) 10:08:55 > barry: cprov: so, it being more readable to do something like "for das in ds.distro_arch_series:" than "for das in ds"? 10:09:01 < bigjools: are you saying __iter__ should be confined to collections? 10:09:38 < cprov: bigjools: being more radical, I think we should never need __iter__ in content classes. 10:09:47 < bigjools: that's what I mean :) 10:09:47 > barry: bigjools: i was going to say, object's that have only one natural iteration, but i think your rule is better 10:10:06 < cprov: bigjools: iterate of a multiple-join (ish) property. 10:10:31 < cprov: s/of/over, sorry. 10:11:27 > barry: cprov: i think +1. you'll never be confused by naming the property explicitly. i can only think that maybe it would be okay for IFnordSet to have an __iter__ that iterates over IFnords 10:11:45 < sinzui: I agree 10:12:01 < cprov: agreed 10:12:12 > barry: beauty. any objections? 10:12:29 > barry: 5...4...3...2...1 10:12:36 > barry: [TOPIC] * mkstemp()/open() combination leaks file-descriptors [cprov] 10:12:37 < MootBot: New Topic: * mkstemp()/open() combination leaks file-descriptors [cprov] 10:12:46 > barry: (got a few for cprov today :) 10:13:23 < cprov: barry: everyone know mkstemp returns and open file descriptor, right ? 10:13:46 > * barry does 10:13:55 < flacoste: fire 10:14:14 < cprov: barry: and if it is not bound to the file you open afterwards it won't be automatically released by python. 10:14:23 < cprov: when you close the file. 10:14:32 < flacoste: why use mkstemp? 10:14:49 < flacoste: i usually prefer creating a temporary directory 10:14:52 < flacoste: and creating files in it 10:15:00 < flacoste: and removing the directory at end of test 10:15:37 < flacoste: using addCleanup 10:15:56 > barry: flacoste: yes, and if you need a tempfile and don't want to hassle with a directory, you should probably close the fd right after mkstemp() and reopen the file name with open() 10:16:09 < cprov: flacoste: that's a good point for testing use-cases 10:16:38 < cprov: flacoste: soyuz uses temp files in production workflows as well. 10:16:49 > barry: <cough>with-statement</cough> :) 10:17:37 < bigjools: how's that python2.6 readiness coming along then barry? :) 10:17:48 > * barry looks at gary_poster and flacoste 10:17:50 < gary_poster: it's coming. :-) 2.5 first 10:17:58 < wgrant: The 2.5 status is actually pretty good. 10:18:06 < wgrant: Mostly benign failures. 10:18:27 < bigjools: then let's JFDI 10:18:35 > barry: my new machine arrives today (hopefully) and i'm going straight to karmic. i'll be itchin' and bitchin' for 2.6 :) 10:18:53 < cprov: anyway, the message I was trying to pass is: be aware of msktemp() ... open() callsites. They should be mkstemp()...os.close(fd) 10:18:54 < allenap: barry: Or use os.fdopen(). 10:19:05 < sinzui: benign failures? Does it apologise before going belly up? 10:19:12 -!- Ursinha:19:22 > barry: allenap: yep; though i almost always want the filename for various purposes 10:20:19 > barry: cprov: +1, thanks. so, when you're reviewing code, watch out for mkstemp(), mkdtemp(), open() or anything else that acquires external resources. ask your dev "are you properly releasing this resource, and if so, where?" 10:20:42 < allenap: barry: I think you get that with fdopen though? It returns a file object for the fd, and the second element of the tuple returned from mkstemp() is the filename. 10:21:42 > barry: allenap: yep, if you want the file object and the filename 10:21:56 > barry: cprov: thanks for this topic. i have one more for ya 10:22:09 > barry: [TOPIC] * Guidelines changes for tests checking systems pre-requisites (specific umasks, app configurations) [maxb, cprov] 10:22:10 < MootBot: New Topic: * Guidelines changes for tests checking systems pre-requisites (specific umasks, app configurations) [maxb, cprov] 10:23:08 < cprov: IIRC, it was specifically about our test suite running on system with use a non-default umask (022) 10:23:46 < wgrant: (and the devscripts config issue) 10:23:53 < cprov: in lp.archivepublisher there as few tests that check files permission 10:24:15 > barry: cprov: well, that's under user control, right? or does our test suite set the umask? are you aware of specific umask failures in our test suite? i remember fixing a bunch of those when i first came on board because i use umask 0002 10:25:34 < cprov: barry: I don't remember exactly which tests failed for maxb. Do you run the test suite locally frequently ? 10:25:45 < * maxb:26:01 < maxb: and reads scrollback 10:26:30 > barry: cprov: i can't get through the whole thing on my current dev box. we'll see about my new one (drive faster fedex!) 10:26:50 < maxb: barry: umask 0002 currently breaks some (one?) soyuz test 10:27:02 > barry: in general though, i would say that any test that is umask sensitive is broken 10:27:19 < wgrant: There is only one. 10:27:25 > barry: maxb: is there a bug on that? imo, it's a broken test 10:27:52 < maxb: The point of the action item is to suggest that the test guidelines should stipulate either that tests should set the appropriate environment, or should check and error clearly if they can't 10:28:07 < cprov: barry: right, I agreed, the test should adjust umask to the expected value. 10:28:11 < maxb: I don't think there's a bug at present, /me scribbles note 10:28:16 > barry: maxb, cprov +1 10:28:35 < sinzui: the tests or the code should set the mask? if there is a test for the mask, I believe the code should set it 10:28:53 > barry: so, as you review branches, be aware of code that might be sensitive to environmental issues, and question whether the tests are fragile because of that 10:29:42 > barry: sinzui: agreed 10:30:51 > barry: cprov, maxb thanks for bringing this up. anything else on this topic? 10:31:27 < cprov: barry: possibly, there is a pending change in our guideline. 10:31:50 < cprov: barry: we use to rely on a pristine ubuntu env for testing and deploying LP 10:32:30 < cprov: barry: from what we've discussed code sensitive to environment changes should set/ensure the expected configurations now. 10:32:34 > barry: cprov: but umask is really part of the user env, not ubuntu env 10:33:03 < flacoste: and even relying on a prisitine ubuntu env is broken 10:33:05 < cprov: barry: right, extent ubuntu env to 'default ubuntu user env' 10:33:16 < maxb: Relying on a pristine environment to run the testsuite is developer-unfriendly 10:33:21 < flacoste: exactly 10:33:40 > barry: cprov: would you take an action item to update our guidelines with this change? 10:33:55 < cprov: right, and that's the change that should be included in our guidelines. 10:34:05 < cprov: barry: yes, sure. 10:34:12 > barry: cprov: great, thanks. 10:34:41 > barry: [ACTION] cprov to update guidelines to clarify how code sensitive to env changes should be written 10:34:43 < MootBot: ACTION received: cprov to update guidelines to clarify how code sensitive to env changes should be written 10:35:08 > barry: thanks again for bringing this up 10:35:20 > barry: [TOPIC] * Invalid markup; The <script> tag requires a close tag (Do not: <script type="text/javascript" src="..."/>) [Curtis] 10:35:22 < MootBot: New Topic: * Invalid markup; The <script> tag requires a close tag (Do not: <script type="text/javascript" src="..."/>) [Curtis] 10:35:31 < sinzui: The HTML 4.x spec requires that the script element have a close tag. 10:35:36 < sinzui: Do not do this: <script type="text/javascript" src="..."/> because the template will not fix our mistake 10:35:55 < sinzui: I saw the compact form in the DSP index and added the issue to todays agenda 10:36:14 < intellectronica: can we lint for this? 10:36:20 < sinzui: two hours later cprov reported that half the content of the apge was missing. I closed the tag and made it work 10:36:31 < sinzui: intellectronica: We need a HTML processor 10:36:37 < * sinzui:36:45 < intellectronica: we currently use xmllint, but we miss the opportunity to automatically discover things that are html 10:37:12 < sinzui: We use a bastardised version my navellint script that I wrote 7 years ago 10:37:23 < noodles775: wont a standard (x)html validator pick that up? (and help us get all our pages valid?) 10:37:36 < sinzui: I am now using an all python script that has a HTML validator 10:37:41 < noodles775: :) 10:38:10 < sinzui: noodles775: comtact is always correct for XML. this is an issue of schema/DTD validity 10:38:34 < sinzui: noodles775: and this is really hard given that PT is generating HTML, it is not HTML 10:38:47 < noodles775: sinzui: yes, but I thought the w3c xhhtml validator pointed ... ah, ok. 10:39:48 < noodles775: So we could validate pages as part of our stories - but that's a separate topic I guess. 10:40:10 < sinzui: We know this has not happened often because Forefox hates it. The issue was fixed within a day of it landing. 10:40:32 < sinzui: noodles775: That is not a story though. 10:40:40 > barry: sinzui: is it just the <script> tag or other tags too? 10:40:47 < sinzui: We have a checker that runs over stanging 10:40:56 < noodles775: True (just convenient given we've rendered the content) 10:41:07 < sinzui: <script> is the only one that comes to mind. 10:41:23 > barry: cool 10:41:24 < sinzui: noodles775: I do not 10:41:54 < sinzui: noodles775: I render content in view tests. stories just hop from link to link and verify messages that the user sees 10:42:40 < noodles775: sinzui: yes - I don't mean that you output the content in the story (urgh), just that the time taken to render the content has already happened when you call click(). 10:42:59 < sinzui: noodles775: another way of putting the issue is that users do not look at the source, we only care about the test that is seen and the links that are traversed 10:43:29 < noodles775: Yes, I agree - it's not the place for validation. 10:43:37 < intellectronica: i really don't think we should do validation in tests. that's what lint is fo 10:43:59 < sinzui: noodles775: given that content/markup is conditional in most templates, that does is using luck to test 10:44:24 > barry: we also often get false positives when linting our pts 10:44:39 < flacoste: we could have a builder runs the test suite in validation mode 10:44:45 < noodles775: intellectronica: sure, but if we wanted to do that, it would mean that lint would need render templates. It would be easier to simply run an external validator over lp.net, I think. 10:44:55 < sinzui: intellectronica: I agree. We want to trust the template engine makes sane markup. that requires up to create sane templates. This is really an issue where HTML 4.x is not XML. 10:46:02 > barry: folks. we're out of time. i don't think we'll solve this here, although let's keep sinzui's specific recommendation in mind. we can talk more about pt validation on list 10:46:06 < sinzui: There is also the issue were we do this: <p></p>; which is also invalid. a <p> must contain content 10:46:12 -!- lifeless: 148 (No route to host)] 10:46:42 > barry: #endmeeting
asiapac
19:00:43 > barry: #startmeeting 19:00:44 < MootBot: Meeting started at 18:00. The chair is barry. 19:00:44 < MootBot: Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] 19:00:50 -!- bigjools:00:54 > barry: jml, rockstar, mwhudson hi! 19:01:02 < mwhudson: barry: hello 19:01:04 < rockstar: Hi barry 19:01:16 < jml: hi 19:01:38 > barry: so, shall i do a quick update from ameu? 19:01:58 < jml: yes please 19:02:00 > barry: there was no ui meeting on monday, so nothing to report there 19:02:09 > barry: * `__iter__()` in model objects? 19:02:29 > barry: we agreed that __iter__ should be avoided except in the case of IFnordSets iterating over IFnords 19:02:40 > barry: i should say "except possibly" 19:02:51 > barry: no mandate, but it's okay 19:03:01 < jml: what was the reasoning? 19:03:26 > barry: for things that aren't naturally collections, it's hard to know from looking at the call site what you're iterating over 19:04:05 > barry: cprov had a specific example that came up in a review and __iter__() just made things harder to understand 19:04:32 > barry: * mkstemp()/open() combination leaks file-descriptors 19:05:00 > barry: reviewers have to watch out for these calls when the code does not clearly close the open fd they return 19:05:01 < rockstar: Ah yes. I reviewed a fix for this last week. 19:05:19 > barry: general call for questioning the opening/leaking of resources 19:05:34 > barry: when you see it in a review, make sure that the resource is being closed 19:06:10 < jml: barry, one nice workaround in test code is to have a wrapper method that calls mkstemp and then calls addCleanup appropriately 19:06:30 < jml: and then returns what mkstemp returns 19:06:35 > barry: jml: right. that was brought up (or something along those lines) 19:06:42 < jml: naturally, that won't stop prod leaks 19:07:45 > barry: flacoste suggested using mkdtemp and then creating files as needed in the directory, blowing the whole dir away in the cleanup 19:08:04 < jml: I tend to do that. 19:08:17 > barry: yep 19:08:48 > barry: * Guidelines changes for tests checking systems pre-requisites (specific umasks, app configurations) 19:09:03 > barry: this came up because maxb got bit by a umask sensitivity in the tests 19:09:21 > barry: and i remembered fixing a bunch of these ages ago when i first joined 19:09:28 > barry: (cause my umask isn't 022) 19:10:03 > barry: the general rule is, if the code requires a specific umask, then put it in the code and test for it, otherwise fiddle the umask in the test 19:10:24 > barry: the general guideline is, don't assume the user env is the default ubuntu user env 19:10:26 < jml: ok. 19:10:36 < mwhudson: i guess the hard part is that most of the time you won't realize you're depending on the umask 19:11:03 < mwhudson: but it's a good principle i guess 19:11:11 > barry: mwhudson: right. i think in general we're pretty good right now. maxb found one failing test in archives which was checking a file perm, and thus relying on umask 19:11:47 < mwhudson: oh right 19:11:55 > barry: * Invalid markup; The <script> tag requires a close tag (Do not: <script type="text/javascript" src="..."/>) 19:12:07 < maxb: the other aspect was invocations of devscripts tools which could be affected by ~/.devscripts. I'm going to do some grepping and liberally sprinkle --no-conf around 19:12:14 < jml: well, the more likely mistake is code that fails because it implicitly relies on a umask and is not tested 19:12:23 > barry: maxb: ah, i didn't know about that one 19:12:42 < jml: maxb, I didn't know about that _directory_ 19:13:10 > * barry neither 19:13:31 < * mwhudson:13:36 < mwhudson: maxb: tell us, what does it do? :) 19:14:10 < mwhudson: barry: shouldn't invalid <script> tags be caught by make lint? 19:14:22 < maxb: directory? ~/.devscripts is a conf file for tweaking the behaviour of the devscripts tools 19:14:32 < mwhudson: oh right 19:14:52 < maxb: when you tweak the behaviour of dch.... the tests get a bit upset :-) 19:15:11 < jml: oh right. 19:15:12 > barry: mwhudson: problem is, our lint only checks the templates, not the generated html 19:15:47 < mwhudson: barry: oh yes 19:16:05 < mwhudson: barry: and we have to generate <script> tags in strange ways because TAL*(&(*&(*&)(* 19:17:06 < jml: so! 19:17:07 > barry: right. curtis gave some good background on what we do now, and what he hopes to do later. there was some discussion about validating the html in the tests, which nobody much liked, validating on staging (or was it edge), etc. no resolution for what we'd eventually like to do 19:17:33 > barry: so reviewers just have to watch out for that 19:17:46 > barry: and empty <p></p> which apparently ain't kosher either 19:17:56 > barry: that's it for ameu update 19:18:02 < jml: yay 19:18:04 < mwhudson: barry: is chameleon brain dead in the same way in this respect? 19:18:17 < jml: barry, one thing on the __iter__ guidelines. 19:18:18 > barry: mwhudson: didn't come up, and i don't know 19:18:26 > barry: jml: k 19:18:40 < mwhudson: jml: operator overloading is evil, don't do it? 19:18:43 < * mwhudson:18:43 < jml: barry, I'd prefer it if we wrote that up as "don't use __iter__ for things that aren't obviously collections" 19:19:02 < jml: mwhudson, operator overloading is great. 19:19:23 > barry: jml: that's how bigjools worded it, which seemed perfect to me 19:19:30 < jml: barry, cool. 19:19:39 < mwhudson: jml: when correctly applied, yes 19:19:55 < jml: barry, I was a little worried about restricting it to IFooSet or whatever. 19:19:56 < mwhudson: branch_set[id] was not a good application :) 19:19:59 > barry: so. i will now open the floor to y'all. anything on your mind today? 19:20:00 < jml: mwhudson, agreed. 19:20:25 < * jml:20:38 > barry: jml: ga-head 19:20:45 < jml: two things 19:21:10 < jml: the first is that I don't know the canonical page to get code review policy from... 19:21:13 < jml: is https://dev.launchpad.net/StyleGuides is ? 19:21:53 < jml: if so, it's a lot of stuff 19:22:42 < jml: the second is that I've been telling people to use the cover letter template as checklist, and not feel obliged to fill in every section. 19:23:31 < jml: which may have been overstepping the mark 19:23:33 > barry: jml: #1 do you mean canonical page for our coding guidelines, or for requesting reviews? 19:23:43 < jml: barry, guidelines 19:23:55 < jml: barry, what I have to look at as a reviewer / patch submitter to see if a patch passes. 19:24:28 > barry: yeah, that's the page, although gary and i have an action item we suck at to migrate stuff on the internal wiki (and old old internal wiki) into here. it would be nice to find some time to clean those pages up too, but don't hold your breaht 19:24:31 > barry: er, breath 19:25:05 < mwhudson: barry: go go go! 19:25:06 < mwhudson: :) 19:25:17 < jml: barry, I understand the difficulty 19:25:23 < jml: and it sure is a tedious task 19:26:02 > barry: jml: but i understand the need to clarify and make concise. seems like a general problem with wiki info (he says nearly completing this month's chr penance) 19:26:15 < jml: but at the rate we are accruing legislation, I'd be quite surprised if any reviewer had everything in their head... 19:26:24 > * barry does :) 19:26:40 < jml: barry, problem is, how can we tell :) 19:26:47 > barry: jml: just ask me :) 19:26:52 < jml: haha 19:26:54 > barry: but that's a joke. i really don't either 19:27:38 > barry: jml: i wouldn't be surprised to see a lot of stuff slip through, but at least if either the dev or reviewer remembers, they have a place to look 19:27:39 < mwhudson: barry: can i call you at 4am to ask for clarification? 19:27:57 > barry: mwhudson: sure. your 4am 19:28:04 < mwhudson: barry: :) 19:28:06 > barry: :) 19:28:23 < jml: barry, having those pages cleaned up would be great. 19:28:43 > barry: jml: seriously though, i'm very sympathetic, i just don't have a good answer 19:28:56 < jml: np 19:29:11 < * mwhudson:29:16 < jml: the other thing was the cover letter 19:29:42 > barry: cover letter, right! do you use the lpreview-body bzr plugin? 19:29:47 < jml: no, I don't. 19:30:03 < jml: but also, I don't tell other people to do so. 19:30:15 > barry: it's pretty simple, but it gives you a standard template in the mailer of you choice, and i do think it's a good idea to fill everything out 19:30:18 > barry: it's also not hard 19:30:47 < jml: I agree with the spirit of it 19:30:59 > barry: jml: but... ? 19:31:15 < jml: but I don't feel like telling community contributors to fill out another form before I review their patch 19:31:33 < jml: and I'm happy enough if people demonstrate that they've thought about each of the items on https://dev.launchpad.net/QuickCoverLetterTemplate 19:31:43 > barry: jml: that's a good point. we get paid to do menial tasks :) 19:31:57 < mwhudson: i think the point is that the reviewee gets the information to the reviewer 19:32:05 < mwhudson: the cover letter template is a handy checklist, no more 19:32:12 < jml: mwhudson, thank you :) 19:32:17 < jml: barry, what he said. 19:32:24 < mwhudson: checklists are great, btw 19:32:29 < rockstar: Just mentioning you've thought about it should suffice. 19:32:30 < * jml:32:48 > barry: agreed! i'm all for streamlining the process, especially for community contributors 19:32:56 < * rockstar:33:15 < jml: barry, as for me, I like using the web ui, just so I can be using the web ui :) 19:33:20 < mwhudson: sounds like resounding agreement 19:33:33 < jml: yay 19:33:51 < * jml:34:01 > barry: yep. though i do have to say that when i'm reviewing i find a filled out cover letter to be very helpful. tbh, when i'm submitting a branch too 19:35:02 > barry: there might also be a difference between the ameu workflow and asiapac workflow, as in how interactive you can be, how big the queue is, etc. 19:35:10 > barry: so i say, use what works for you 19:35:17 < jml: +1 19:35:52 < mwhudson: i think we should certainly recommend the cover letter 19:36:31 > barry: mwhudson: i'd go a little further. if you're queuing up for +activereviews, a cover letter is much more important. if you're doing a highly interactive one, much less so 19:36:41 < mwhudson: barry: right 19:36:59 > barry: as you say, the important thing is to communicate the info, however that happens 19:37:24 < jml: btw 19:37:27 < jml: putting it out there 19:37:48 < jml: a flagon of $BEVERAGE to whoever makes a tool that submits a branch to Launchpad via ec2test based only on the MP page. 19:38:17 > barry: tarmac? 19:38:21 < rockstar: jml, you mean like Tarmac? 19:38:29 < jml: rockstar, I don't know! 19:38:32 < rockstar: I'm sure we could probably do something like that. 19:38:43 < rockstar: jml, let's talk after this. 19:38:53 < mwhudson: jml: get rockstar to do it, he won't ask for a flagon of single malt 19:38:59 > barry: rockstar: is a switch to tarmac in the cards after 3.0? 19:39:21 < rockstar: barry, well, lp-oops is using it, and some ubuone folks. We're working out kinks still. 19:39:22 < mwhudson: abentley asked in today's call what PQM still does for us 19:39:42 < rockstar: abentley broke Tarmac by changing the API - another hint that we need a versioned API. 19:39:45 < mwhudson: over and above us just merging and pushing 19:39:45 > barry: mwhudson: that's easy. it crashes and obfuscates 19:39:52 < rockstar: :) 19:40:02 < jml: it checks for db changes in devel 19:40:07 < jml: it enforces the testfix thing 19:40:15 < rockstar: We can do this all very easily in Tarmac. 19:40:19 < jml: (whether that is _for_ us or _against_ us is an open question) 19:40:40 < rockstar: If all we're doing is merging, I'm not even sure why PQM does a build at all. 19:40:49 > barry: testfix thing: hopefully much more transparently than pqm 19:41:04 > barry: pqm does /not/ run in 5 minutes 19:41:17 < rockstar: I think Tarmac will be a lot more useful when we have merge queues. 19:41:39 > barry: btw, did someone make ec2 --headless detach more quickly recently? 19:41:54 < jml: not I. 19:42:14 < mwhudson: barry: a new AMI for ec2test will help a lot there 19:42:19 < mwhudson: barry: i intend to do that today 19:42:47 > barry: fantastic. it did not seem to take 45m the last time i used it (more like 5-10 which was a pleasant surprise) 19:43:00 > barry: could have been luck i s'pose 19:43:00 < mwhudson: two things are very variable 19:43:06 < mwhudson: 1) instance launch time 19:43:09 < jml: rockstar, what I want is "submit https://code.edge.launchpad.net/~wgrant/launchpad/whatever/+merge/212", for it to take the commit message, target branch and reviewer info and submit it to ec2test. 19:43:10 < mwhudson: 2) make schema time 19:43:20 < jml: rockstar, tarmac's README implies it does something else. 19:43:24 < mwhudson: (making schema before detaching is insane, but...) 19:43:46 > barry: mwhudson: yep. would be nice to detach asap 19:43:55 < rockstar: jml, yea, like I said, we should talk after the meeting. I'm sure my documentation skills are pure suckage. 19:44:11 < mwhudson: barry: i hope to get to that before I run out of BE time 19:44:13 < * jml:) 19:44:17 > barry: rockstar: use sphinx 19:44:20 < mwhudson: (or sanity, whichever comes first) 19:44:27 > barry: jml: yes. i think we're done. any objections? 19:44:31 < mwhudson: nope 19:44:33 < rockstar: Nope. 19:44:37 > barry: #endmeeting