[15:00] <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:01] <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:02] <sinzui> me
[15:02] <barry> [TOPIC] agenda
[15:02] <MootBot> New Topic:  agenda
[15:02] <barry> * Roll call
[15:03] <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:04] <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:05] <gary_poster> (done, in case it wasn't clear :-) )
[15:05] <barry> gary_poster: sorry i think they're less readable :)
[15:06] <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:07] <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:08] <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:10] <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:11] <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:12] <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:13] <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:14] <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:15] <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:16] <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:17] <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:18] <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:19] <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:20] <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:21] <abentley> gary_poster: niemeyer was not receptive to my suggestions.
[15:21] <gary_poster> abentley: yes, definitely observed
[15:22] <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:23] <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:24] <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:25] <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:26] <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:27] <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:28] <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:29] <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:30] <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:31] <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:32] <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:33] <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:34] <barry> abentley: what are you waiting for? :)
[15:35] <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:36] <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:37] <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:38] <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:39] <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:40] <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:41] <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:42] <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:43] <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:44] <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:45] <mars> Hi everyone
[15:45] <mars> Last Monday the AJAX team decided to expand the scope of JavaScript reviews in Launchpad.
[15:45] <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:46] <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:47] <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:48] <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:49] <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:50] <abentley> intellectronica: Not every branch related to a bug fixes that bug.  Some are incremental branches.
[15:51] <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:52] <sinzui> mars I certain will