| === Ursinha is now known as bananinha | ||
| === bananinha is now known as Ursinha | ||
| === Ursinha is now known as Ursinha-afk | ||
| === mrevell is now known as mrevell-lunch | ||
| === Ursinha-afk is now known as Ursinha | ||
| === mrevell-lunch is now known as mrevell | ||
| barry | #startmeeting | 15:00 |
|---|---|---|
| MootBot | Meeting started at 09:00. The chair is barry. | 15:00 |
| MootBot | Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE] | 15:00 |
| barry | hello everyone and welcome to this week's ameu reviewer's meeting. who's here today? | 15:00 |
| gmb | me | 15:00 |
| gmb | ... and apparently no-one else... | 15:01 |
| bac | me | 15:01 |
| mars | me | 15:01 |
| gary_poster | me | 15:01 |
| danilos | me | 15:01 |
| flacoste | me | 15:01 |
| rockstar | me | 15:01 |
| salgado | me | 15:01 |
| adeuring | me | 15:01 |
| sinzui | me | 15:02 |
| barry | [TOPIC] agenda | 15:02 |
| MootBot | New Topic: agenda | 15:02 |
| barry | * Roll call | 15:02 |
| barry | * Reviewers to enforce bug<->branch linking for all non-trivial branches (intellectronica) | 15:03 |
| barry | * Determine official preference, if any, of Storm usage: And(expr, expr) vs. expr & expr, Gary | 15:03 |
| barry | * Determine why _get_store uses protected spelling, and if we think it should, Gary | 15:03 |
| barry | * Merge proposals and cover letters (barry) | 15:03 |
| barry | * Using OCR for JavaScript reviews (mars) | 15:03 |
| barry | * Peanut gallery (anything not on the agenda) | 15:03 |
| barry | * Mentoring update | 15:03 |
| barry | * Action items | 15:03 |
| BjornT | me | 15:03 |
| bigjools | me | 15:03 |
| barry | intellectronica: ping | 15:03 |
| barry | i guess we'll skip his topic for now | 15:03 |
| barry | [TOPIC] * Determine official preference, if any, of Storm usage: And(expr, expr) vs. expr & expr, Gary | 15:04 |
| MootBot | New Topic: * Determine official preference, if any, of Storm usage: And(expr, expr) vs. expr & expr, Gary | 15:04 |
| barry | gary_poster: you're up | 15:04 |
| gary_poster | Storm supports both imported And/Or (e.g., ``And(expr, expr[, expr]*)``) and overloaded binary operators (e.g., ``(expr) & (expr) [& (expr)]*``). | 15:04 |
| gary_poster | See https://storm.canonical.com/Manual#AND . | 15:04 |
| gary_poster | In our codebase I see And/Or, not binary operators. | 15:04 |
| gary_poster | I have a branch going through review that uses the binary operators. | 15:04 |
| gary_poster | My opinions... | 15:04 |
| gary_poster | Positives for binary operators: | 15:04 |
| gary_poster | - no imports | 15:04 |
| gary_poster | - readable | 15:04 |
| gary_poster | Negatives for binary operators: | 15:04 |
| gary_poster | - a hack (they should be able to overload boolean operators but Python does not support) | 15:04 |
| gary_poster | - because of hack, precedence is wrong (e.g., ``Class.foo==42 & Class.bar=='kumquat'`` does not work; you need ``(Class.foo==42) & (Class.bar=='kumquat')``). | 15:04 |
| gary_poster | Maybe some will not find it as readable and stick "less readable" in the negatives. | 15:04 |
| gary_poster | So, what is status of using overloaded binary operators for combining Storm expressions? Preferred, rejected, allowed, or allowed-while-we-evaluate? | 15:04 |
| gary_poster | (done, in case it wasn't clear :-) ) | 15:05 |
| barry | gary_poster: sorry i think they're less readable :) | 15:05 |
| danilos | agreed | 15:06 |
| gmb | Ditto. | 15:06 |
| barry | gary_poster: and the precedence problem worries me | 15:06 |
| BjornT | -1 on using binary operators | 15:06 |
| gary_poster | gary_poster: cool. | 15:06 |
| bigjools | precendece is my main issue | 15:06 |
| adeuring | -1 -- the precende problem can cause nightmares... | 15:06 |
| gary_poster | (nightmares--well, not really, at least IME--you just get a clear failure) | 15:07 |
| gary_poster | ok, so done? | 15:07 |
| barry | gary_poster: i think so, sorry ;) | 15:07 |
| gary_poster | np | 15:07 |
| intellectronica | me (sorry for lateness - new wireless router) | 15:07 |
| allenap | me | 15:07 |
| barry | intellectronica: we'll come back to your topic | 15:07 |
| barry | [TOPIC] * Determine why _get_store uses protected spelling, and if we think it should, Gary | 15:08 |
| MootBot | New Topic: * Determine why _get_store uses protected spelling, and if we think it should, Gary | 15:08 |
| danilos | intellectronica: it has some serious latency, you might want to consider going back to the old one | 15:08 |
| gary_poster | Last week we stated that we should not have public methods spelled as protected methods (_*). | 15:08 |
| gary_poster | I noticed that our SQLBase bits seem to rely on ``_get_store``. | 15:08 |
| gary_poster | This method can be convenient even when using Storm spelling. It is shorter to use than the full utility call, and it specifies which store and flavor to use. | 15:08 |
| gary_poster | I'm guessing that changing this means changing our SQLBase code? | 15:08 |
| gary_poster | I'm inclined to put changing this as a bug report in LP. | 15:08 |
| gary_poster | Agree/appropriate/possible? | 15:08 |
| gary_poster | (Or maybe I misunderstand about this being needed by our SQLBase stuff?) | 15:08 |
| gary_poster | (done) | 15:08 |
| barry | gary_poster: have you seen abentley's branch? | 15:10 |
| danilos | is this not about storm code? | 15:10 |
| gary_poster | barry: no | 15:10 |
| flacoste | so i asked jamesh | 15:10 |
| flacoste | and he said that the only reason for having an underscore there | 15:11 |
| flacoste | is that it's not part of the SQLObject API | 15:11 |
| flacoste | but I don't think we care now | 15:11 |
| barry | abentley has a new storm base class for convenience. i think jml was reviewing that but i don't know the status of it | 15:11 |
| barry | it provides a convenient way to get the store | 15:11 |
| barry | flacoste: i think you're right. plus we want to get rid of the sqlobject api anyway don't we? :) | 15:12 |
| flacoste | well | 15:12 |
| flacoste | sometime in the future | 15:12 |
| flacoste | but it's a lot of work | 15:13 |
| flacoste | and not that practical either | 15:13 |
| flacoste | and the bare storm API is a pain | 15:13 |
| gary_poster | heh | 15:13 |
| flacoste | so SugarStorm is nice | 15:13 |
| barry | we can do it piecemeal too right? | 15:13 |
| flacoste | more or less | 15:13 |
| barry | right, that's what i'm thinking | 15:13 |
| flacoste | because of the incompatibility between the resultset interface | 15:13 |
| rockstar | flacoste, bare Storm needs improvements, but if we avoid those improvements, that's a tragedy. | 15:13 |
| flacoste | and we use these for unions | 15:13 |
| flacoste | i have great hope for abentley's SugarStorm | 15:14 |
| flacoste | or watherver it's called | 15:14 |
| gary_poster | agree with rockstar. If this is a Storm thing, then let's change this in Storm. This seems like it should go lower than SugarStorm | 15:14 |
| flacoste | we should probably make it easy to combine SQLObject and Storm ResultSet | 15:14 |
| flacoste | so that we aren't block by the incompatibility | 15:14 |
| flacoste | because when you start converting one object | 15:15 |
| barry | flacoste: +1 if possible | 15:15 |
| flacoste | which uses UNION | 15:15 |
| flacoste | it becomes an never-ending tasks | 15:15 |
| flacoste | I once tried converting Person to use only the Store API | 15:15 |
| flacoste | and gave up because the branch was inflating faster than Argentina's inflation | 15:15 |
| gary_poster | heh | 15:15 |
| rockstar | :) | 15:15 |
| barry | flacoste: 70 hits for 'union|UNION' in lib/c/l/database | 15:16 |
| bigjools | I got caught by the union thing | 15:16 |
| bigjools | would be great to fix that | 15:16 |
| barry | flacoste: well, person might not be the best first choice :) | 15:16 |
| flacoste | well, i was there | 15:16 |
| flacoste | and it started fine :-) | 15:16 |
| barry | :-D | 15:16 |
| flacoste | and needed to use storm for some thing | 15:16 |
| flacoste | anyway | 15:16 |
| gary_poster | ( well, the Person anecdote was intended as an example of Storm API not being as convenient, right?) | 15:17 |
| barry | so, back to gary's specific question: what to do about _get_store()? | 15:17 |
| salgado | gary_poster, flacoste, will we still want obj.get_store() after stub's landed his sso branch? it includes some IStore adapters for our classes | 15:17 |
| gary_poster | salgado: if we *have* to have these helper static methods anyway, and they are so easy to use, shouldn't we just use them? | 15:18 |
| gmb | herb: Okay, can you apply https://pastebin.canonical.com/14232/ and then apply https://pastebin.canonical.com/14233/ please? | 15:18 |
| gmb | Heh. | 15:19 |
| gary_poster | gmb: hee hee | 15:19 |
| gmb | Wrong channel. | 15:19 |
| abentley | salgado: I certainly prefer class methods over zope stuff where either solution works. | 15:19 |
| gary_poster | agree | 15:19 |
| salgado | fair enough. I prefer that too | 15:19 |
| gary_poster | so to change this is this in our code base or in Storm? flacoste ? | 15:20 |
| gary_poster | 'cause if it is in Storm, may or may not happen ... | 15:20 |
| abentley | gary_poster: niemeyer was not receptive to my suggestions. | 15:21 |
| gary_poster | abentley: yes, definitely observed | 15:21 |
| === Ursinha is now known as aham | ||
| === aham is now known as Ursinha | ||
| barry | abentley: did jml finish the review of your branch? | 15:22 |
| abentley | barry: No. He asked for changes, I submitted a revised version, then nothing. | 15:22 |
| barry | abentley: i'll ping him tonight about it | 15:22 |
| barry | abentley: but you should too :) | 15:22 |
| abentley | barry: I've got plenty of other stuff to work on, so I'm not fussed. | 15:23 |
| barry | gary_poster: have we resolved your topic? ;) | 15:23 |
| gary_poster | barry: not really, unless we say that, because there wasn't much clamoring of agreement with me that this ought to be changed, we should just ignore it. | 15:24 |
| gary_poster | but then, can I use _get_store? | 15:24 |
| barry | gary_poster: you can always add an alias for get_store() and use that | 15:25 |
| gary_poster | My branch does. flacoste thouht it was reasonable at the time. | 15:25 |
| barry | e.g.: get_store = _get_store | 15:25 |
| barry | sounds good to me | 15:25 |
| salgado | should it not be getStore(), though? | 15:25 |
| gary_poster | and we tack that on to all our classes? | 15:25 |
| barry | salgado: dang, yeah :/ | 15:25 |
| barry | gary_poster: you had another topic? | 15:26 |
| gary_poster | heh, yeah. one sec, lemme get the paste | 15:26 |
| gary_poster | I miss the old bzr review plugin a bit because it enforced our policies. | 15:26 |
| gary_poster | In particular, the lint is not being used regularly, and the information about pre-impl call is often missing in reviews I've done. | 15:26 |
| gary_poster | I have a quick example about the ``make lint`` issue. | 15:26 |
| gary_poster | I noticed when I had to manually switch my most recent branch over from devel to db-devel, and merge, that ``make lint`` had apparently not been run for many of the branches. | 15:26 |
| gary_poster | I had to do a lot of cleaning up on the merged code to make ``make lint`` happy. The majority of the complaints were real. | 15:26 |
| gary_poster | We're building generic bzr tools AIUI (bzr send), but our policies are not generic, and enforcing them is valuable. | 15:26 |
| gary_poster | I'm inclined to want a lp-specific bzr plugin that defers the heavy lifting to the bzr send plugin code, but is more specific to what we want. | 15:26 |
| gary_poster | I also wonder if ec2test could help with the linting, but that's not as clear to me. | 15:26 |
| gary_poster | The general idea would be that, if you use -s to submit to PQM, it would make sure that ``make lint`` was happy. | 15:27 |
| gary_poster | However, with the old review plugin, there was a way to say "yeah, yeah, make lint is on crack for that complaint." We would need that here too. | 15:27 |
| gary_poster | We could put it in command line options but would prefer to have it in a file. Not sure where the file would go. But that's an implementation detail. | 15:27 |
| gary_poster | So: IRT my general observation of having regressed a bit in our process because of missing automation for linting and message regexes: agree/disagree? | 15:27 |
| gary_poster | And then furthermore, if you agree that we have regressed, what should we do to improve? | 15:27 |
| gary_poster | (done) | 15:27 |
| bigjools | can we fix lpreview to just use lpsend? | 15:28 |
| gmb | There's no reason we can't hack the existing review plugin to do lint + cover letter and then hand off to bzr send. | 15:28 |
| gmb | Well, I don't *think* there's any reason, anyway. | 15:28 |
| barry | gary_poster: that dovetails with my topic. if we used the standard cover letter template for mps at least people would have to willfully ignore linting and pre-imps | 15:28 |
| gary_poster | gmb: that's the kind of thing I'm talking about, yeah | 15:28 |
| bigjools | gmb: great minds think alike | 15:28 |
| gmb | abentley would know better than I. | 15:28 |
| abentley | gmb: Yes there is. There's no way to specify a message body | 15:28 |
| gmb | abentley: That's what I was afraid you were going to say. | 15:29 |
| abentley | gmb: It's not that 'bzr send' couldn't, it's that it's not implemented yet. | 15:29 |
| barry | abentley: i wonder if there was some $EDITOR hack we could use for that | 15:29 |
| gmb | barry: send just passes over to the MUA, so I'm not sure that would work. | 15:30 |
| abentley | barry: If we want to force everyone to use mail_client=editor, perhaps. | 15:30 |
| barry | abentley: yeah, that's what i meant. and it would just be for lp branches, so location.conf could override | 15:30 |
| intellectronica | abentley: i use gmail, and so i benefit from jamesh's plugin. having to use EDITOR will make like hard for me | 15:30 |
| barry | abentley: not that i have any concrete ideas. just wonderin' | 15:30 |
| sinzui | I would disable such a feature because I start my cover letter when I create my branch | 15:31 |
| gary_poster | barry: yeah, some kind of automated/enforced-ish standard template would be fine with me too. It's not as tight as the lpreview approach, but something is better than nothing, and might be sufficient | 15:31 |
| danilos | it'd be nice for lpsend to actually work with my name first, I have to manually paste the cover letter anyway | 15:31 |
| barry | sinzui: so do i | 15:31 |
| abentley | danilos: Work with your name? | 15:31 |
| BjornT | danilos: +1 :) | 15:31 |
| gary_poster | so the temolate thing seems like an insufficient idea | 15:31 |
| mars | danilos, +1 as well :) | 15:32 |
| BjornT | abentley: some of us get a unicode error when using lpsend | 15:32 |
| abentley | BjornT: Not from send, from Launchpad, right? | 15:32 |
| danilos | abentley: right | 15:32 |
| BjornT | abentley: right | 15:32 |
| abentley | So that's not a send bug, and AIUI, work has been done to fix it this cycle. | 15:33 |
| abentley | I've said before, but I guess I have to say again, I think bzr send should support a hook that we can use to plug in the process stuff from lpreview. | 15:33 |
| barry | abentley: what are you waiting for? :) | 15:34 |
| gary_poster | abentley: (I don't remember hearing that before) +1 | 15:35 |
| abentley | barry: a free moment. | 15:35 |
| barry | abentley: those are so overrated | 15:35 |
| gary_poster | ok, so maybe this is another one of those "...and we descend into silence" moments, this time because we agree that something needs to be done but don't have the time. | 15:35 |
| gmb | abentley: Is there a bug for updating send to add that hook? | 15:35 |
| abentley | gmb: I don't know. | 15:36 |
| barry | gary_poster: yep. seems like time is better spend on send rather than lpreview though, so that's something | 15:36 |
| mars | gary_poster, a very astute observation :) | 15:36 |
| gary_poster | :-) | 15:36 |
| barry | gary_poster: will you check to see if there's a bug open on adding a hook for bzr send? | 15:36 |
| gary_poster | barry: ok | 15:36 |
| barry | [ACTION] gary_poster will check to see if there's a bug open for adding a hook to 'bzr send' and submit one if there isn't | 15:37 |
| MootBot | ACTION received: gary_poster will check to see if there's a bug open for adding a hook to 'bzr send' and submit one if there isn't | 15:37 |
| gary_poster | barry: IRT my previous topic: sorry, but I was not clear. so can I just use _get_store for now, and move on? | 15:37 |
| barry | gary_poster: add a getStore() alias if possible and use that | 15:37 |
| gary_poster | ...ok | 15:38 |
| barry | thanks | 15:38 |
| gary_poster | sure, thanks everybody | 15:38 |
| barry | [TOPIC] * Merge proposals and cover letters (barry) | 15:38 |
| MootBot | New Topic: * Merge proposals and cover letters (barry) | 15:38 |
| barry | so along the lines of gary_poster 's last topic... | 15:38 |
| barry | i notice that not everyone is using the standard cover letter template. some people don't use any template | 15:38 |
| barry | should we enforce that? (updating the template if necessary) | 15:39 |
| * mars hides | 15:39 | |
| * gary_poster hides | 15:39 | |
| barry | not blaming anyway, just wondering! | 15:39 |
| * danilos hides behind gary | 15:39 | |
| gary_poster | :-) | 15:39 |
| gary_poster | lol | 15:39 |
| * BjornT doesn't know where the standard template is located | 15:39 | |
| barry | if the template isn't useful or overkill, let's say so | 15:39 |
| danilos | I don't know where it is either, though | 15:39 |
| abentley | I don't think we should enforce a template. There are lots of times it doesn't make sense. | 15:39 |
| barry | https://dev.launchpad.net/QuickCoverLetterTemplate | 15:40 |
| BjornT | i think we should get bzr send to use the template. then people will automatically use it, unless they explicitly choose not to | 15:40 |
| barry | well, the diff output could be removed now | 15:40 |
| mars | BjornT, +1 | 15:40 |
| barry | abentley: i think the template usually makes sense though | 15:40 |
| mars | a new plugin for bzr send that adds a --template option? | 15:40 |
| gary_poster | I must admit, I kind of like using lp TTW to request reviews | 15:41 |
| gary_poster | but you know, if "we don't do that" that's ok | 15:41 |
| barry | gary_poster: you could still use the template :) | 15:41 |
| gary_poster | but seems a bit of a shame, since we have a TTW interface | 15:41 |
| mars | gary_poster, that's a different problem - communicating a project's review preferences through the web API | 15:41 |
| abentley | barry: I don't. I find I'm usually writing the same thing under several headings. | 15:41 |
| mars | oops, nix the 'API' from that last statement | 15:42 |
| gary_poster | barry: sure...which is where? and how is policy for *that* plugged in? | 15:42 |
| gary_poster | (I mean, in the TTW interface) | 15:42 |
| gary_poster | oh sorry | 15:42 |
| barry | abentley: really? maybe there's some confusion about what's intended for each section? | 15:42 |
| flacoste | i preferred the old template | 15:43 |
| flacoste | and i still use it | 15:43 |
| barry | gary_poster: well, i just mean that you could paste the cover letter into the text area | 15:43 |
| flacoste | i run bzr write-cover-letter | 15:43 |
| abentley | Well, a bit, but it generally just gets in the way of a natural description of the change. | 15:43 |
| flacoste | and then :r cover.txt | 15:43 |
| gmb | flacoste: Me too. | 15:43 |
| flacoste | when bzr send fires my editor | 15:43 |
| gmb | Well, for non-trivial branches, anyway. | 15:43 |
| mars | :r ? | 15:43 |
| flacoste | mars: i see you aren't enlightened | 15:43 |
| flacoste | vim powers! | 15:43 |
| danilos | mars: it's likely just some vi crap, ignore it :) | 15:43 |
| mars | lol | 15:44 |
| mars | ah | 15:44 |
| barry | dang, we're almost out of time. let's drop this for now because i'd like to give mars's topic a shot | 15:44 |
| barry | [TOPIC] * Using OCR for JavaScript reviews (mars) | 15:44 |
| MootBot | New Topic: * Using OCR for JavaScript reviews (mars) | 15:44 |
| mars | barry, I'll be brief | 15:44 |
| barry | mars: thx | 15:44 |
| mars | Hi everyone | 15:45 |
| mars | Last Monday the AJAX team decided to expand the scope of JavaScript reviews in Launchpad. | 15:45 |
| === Ursinha is now known as ra | ||
| === ra is now known as Ursinha | ||
| mars | Any OCR should be able to pick up a JavaScript review, if they feel comfortable doing so. We just ask that the review be unofficially mentored by one of the sprint attendees. | 15:45 |
| mars | These people were at the AJAX sprint, or on the AJAX team, and can mentor a JavaScript review: flacoste, mars, sinzui, Edwin, cprov, noodles, intellectronica, rockstar, danilo, and jtv | 15:45 |
| mars | Also, feel free to ask me if any questions come up during the review. | 15:45 |
| mars | that's all | 15:45 |
| * allenap discovers that TTW means Table-Top Wargaming | 15:45 | |
| gary_poster | lol | 15:46 |
| gary_poster | Through-the-web was how I've heard it used, and my intent :-) | 15:46 |
| bigjools | I thought it was ruder | 15:46 |
| barry | mars: thanks | 15:46 |
| barry | any comments on mars's request or should we break? | 15:47 |
| sinzui | I don't qualify to be a YUI/Windmill reviewer. I just abandoned using windmill to test YUI + GMap. I'm back to despising YUI for reinventing everything to make mashups nigh impossible. | 15:47 |
| gary_poster | write it down someplace | 15:47 |
| allenap | Sounds like a good request. | 15:47 |
| intellectronica | barry: mention my item, since i don't think it requires much discussion :) | 15:47 |
| mars | sinzui, fair enough | 15:47 |
| barry | intellectronica: ah, sorry | 15:47 |
| sinzui | PS. I hate GMap too | 15:47 |
| barry | [TOPIC] * Reviewers to enforce bug<->branch linking for all non-trivial branches (intellectronica) | 15:47 |
| MootBot | New Topic: * Reviewers to enforce bug<->branch linking for all non-trivial branches (intellectronica) | 15:47 |
| mars | sinzui, I would love it if you could write a ranting email about that statement though | 15:47 |
| barry | intellectronica: +1 :) | 15:48 |
| gary_poster | intellectronica: (1) I assume up mean --fixes=lp:XXX? | 15:48 |
| gary_poster | (2) how do we check that? | 15:48 |
| barry | gary_poster: or you can do it TTW :) | 15:48 |
| intellectronica | gary_poster: either that, or you can do it ttw | 15:48 |
| gary_poster | ah ok | 15:48 |
| intellectronica | heh | 15:48 |
| barry | heh | 15:48 |
| gary_poster | :-) | 15:48 |
| sinzui | mars: I was going to bring it up in the meeting, but my thoughts are not collected. | 15:49 |
| intellectronica | and you can check it ttw, or do it after the fact | 15:49 |
| mars | gary_poster, require a hook in pqm-submit, --no-fixes has to be explicitly set for this branch? | 15:49 |
| mars | kinda like explicitly setting --no-lint | 15:49 |
| barry | okay, so apologies for going over. thanks everybody and have a good day | 15:49 |
| intellectronica | mars: the thing is, --fixes is on commit, not on pqm submit | 15:49 |
| intellectronica | thanks barry | 15:49 |
| barry | #ENDMEETING | 15:49 |
| MootBot | Meeting finished at 09:49. | 15:49 |
| abentley | intellectronica: Not every branch related to a bug fixes that bug. Some are incremental branches. | 15:50 |
| intellectronica | abentley: i agree. but they can still be linked to the bug | 15:51 |
| abentley | intellectronica: They can't be --fixes. | 15:51 |
| mars | sinzui, ok, but do let me know when you have something put together. I'd love to read it. | 15:51 |
| intellectronica | abentley: right, but they can be linked ttw with a comment explaining what is their relation to the bug | 15:51 |
| sinzui | mars I certain will | 15:52 |
| === ursula is now known as Ursinha | ||
| === Ursinha is now known as uia | ||
| === uia is now known as Ursinha | ||
| === ursula is now known as Ursinha | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!