/srv/irclogs.ubuntu.com/2009/02/25/#launchpad-meeting.txt

=== 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#startmeeting15:00
MootBotMeeting started at 09:00. The chair is barry.15:00
MootBotCommands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]15:00
barryhello everyone and welcome to this week's ameu reviewer's meeting.  who's here today?15:00
gmbme15:00
gmb... and apparently no-one else...15:01
bacme15:01
marsme15:01
gary_posterme15:01
danilosme15:01
flacosteme15:01
rockstarme15:01
salgadome15:01
adeuringme15:01
sinzuime15:02
barry[TOPIC] agenda15:02
MootBotNew Topic:  agenda15:02
barry* Roll call15: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, Gary15:03
barry * Determine why _get_store uses protected spelling, and if we think it should, Gary15: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 update15:03
barry * Action items15:03
BjornTme15:03
bigjoolsme15:03
barryintellectronica: ping15:03
barryi guess we'll skip his topic for now15:03
barry[TOPIC]  * Determine official preference, if any, of Storm usage: And(expr, expr) vs. expr & expr, Gary15:04
MootBotNew Topic:   * Determine official preference, if any, of Storm usage: And(expr, expr) vs. expr & expr, Gary15:04
barrygary_poster: you're up15:04
gary_posterStorm supports both imported And/Or (e.g., ``And(expr, expr[, expr]*)``) and overloaded binary operators (e.g., ``(expr) & (expr) [& (expr)]*``).15:04
gary_posterSee https://storm.canonical.com/Manual#AND .15:04
gary_posterIn our codebase I see And/Or, not binary operators.15:04
gary_posterI have a branch going through review that uses the binary operators.15:04
gary_posterMy opinions...15:04
gary_posterPositives for binary operators:15:04
gary_poster- no imports15:04
gary_poster- readable15:04
gary_posterNegatives 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_posterMaybe some will not find it as readable and stick "less readable" in the negatives.15:04
gary_posterSo, 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
barrygary_poster: sorry i think they're less readable :)15:05
danilosagreed15:06
gmbDitto.15:06
barrygary_poster: and the precedence problem worries me15:06
BjornT-1 on using binary operators15:06
gary_postergary_poster: cool.15:06
bigjoolsprecendece is my main issue15: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_posterok, so done?15:07
barrygary_poster: i think so, sorry ;)15:07
gary_posternp15:07
intellectronicame (sorry for lateness - new wireless router)15:07
allenapme15:07
barryintellectronica: we'll come back to your topic15:07
barry[TOPIC]  * Determine why _get_store uses protected spelling, and if we think it should, Gary15:08
MootBotNew Topic:   * Determine why _get_store uses protected spelling, and if we think it should, Gary15:08
danilosintellectronica: it has some serious latency, you might want to consider going back to the old one15:08
gary_posterLast week we stated that we should not have public methods spelled as protected methods (_*).15:08
gary_posterI noticed that our SQLBase bits seem to rely on ``_get_store``.15:08
gary_posterThis 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_posterI'm guessing that changing this means changing our SQLBase code?15:08
gary_posterI'm inclined to put changing this as a bug report in LP.15:08
gary_posterAgree/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
barrygary_poster: have you seen abentley's branch?15:10
danilosis this not about storm code?15:10
gary_posterbarry: no15:10
flacosteso i asked jamesh15:10
flacosteand he said that the only reason for having an underscore there15:11
flacosteis that it's not part of the SQLObject API15:11
flacostebut I don't think we care now15:11
barryabentley has a new storm base class for convenience.  i think jml was reviewing that but i don't know the status of it15:11
barryit provides a convenient way to get the store15:11
barryflacoste: i think you're right.  plus we want to get rid of the sqlobject api anyway don't we? :)15:12
flacostewell15:12
flacostesometime in the future15:12
flacostebut it's a lot of work15:13
flacosteand not that practical either15:13
flacosteand the bare storm API is a pain15:13
gary_posterheh15:13
flacosteso SugarStorm is nice15:13
barrywe can do it piecemeal too right?15:13
flacostemore or less15:13
barryright, that's what i'm thinking15:13
flacostebecause of the incompatibility between the resultset interface15:13
rockstarflacoste, bare Storm needs improvements, but if we avoid those improvements, that's a tragedy.15:13
flacosteand we use these for unions15:13
flacostei have great hope for abentley's SugarStorm15:14
flacosteor watherver it's called15:14
gary_posteragree with rockstar.  If this is a Storm thing, then let's change this in Storm.  This seems like it should go lower than SugarStorm15:14
flacostewe should probably make it easy to combine SQLObject and Storm ResultSet15:14
flacosteso that we aren't block by the incompatibility15:14
flacostebecause when you start converting one object15:15
barryflacoste: +1 if possible15:15
flacostewhich uses UNION15:15
flacosteit becomes an never-ending tasks15:15
flacosteI once tried converting Person to use only the Store API15:15
flacosteand gave up because the branch was inflating faster than Argentina's inflation15:15
gary_posterheh15:15
rockstar:)15:15
barryflacoste: 70 hits for 'union|UNION' in lib/c/l/database15:16
bigjoolsI got caught by the union thing15:16
bigjoolswould be great to fix that15:16
barryflacoste: well, person might not be the best first choice :)15:16
flacostewell, i was there15:16
flacosteand it started fine :-)15:16
barry:-D15:16
flacosteand needed to use storm for some thing15:16
flacosteanyway15:16
gary_poster( well, the Person anecdote was intended as an example of Storm API not being as convenient, right?)15:17
barryso, back to gary's specific question: what to do about _get_store()?15:17
salgadogary_poster, flacoste, will we still want obj.get_store() after stub's landed his sso branch?  it includes some IStore adapters for our classes15:17
gary_postersalgado: 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
gmbherb: Okay, can you apply https://pastebin.canonical.com/14232/ and then apply https://pastebin.canonical.com/14233/ please?15:18
gmbHeh.15:19
gary_postergmb: hee hee15:19
gmbWrong channel.15:19
abentleysalgado: I certainly prefer class methods over zope stuff where either solution works.15:19
gary_posteragree15:19
salgadofair enough.  I prefer that too15:19
gary_posterso 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
abentleygary_poster: niemeyer was not receptive to my suggestions.15:21
gary_posterabentley: yes, definitely observed15:21
=== Ursinha is now known as aham
=== aham is now known as Ursinha
barryabentley: did jml finish the review of your branch?15:22
abentleybarry: No.  He asked for changes, I submitted a revised version, then nothing.15:22
barryabentley: i'll ping him tonight about it15:22
barryabentley: but you should too :)15:22
abentleybarry: I've got plenty of other stuff to work on, so I'm not fussed.15:23
barrygary_poster: have we resolved your topic? ;)15:23
gary_posterbarry: 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_posterbut then, can I use _get_store?15:24
barrygary_poster: you can always add an alias for get_store() and use that15:25
gary_posterMy branch does.  flacoste thouht it was reasonable at the time.15:25
barrye.g.:  get_store = _get_store15:25
barrysounds good to me15:25
salgadoshould it not be getStore(), though?15:25
gary_posterand we tack that on to all our classes?15:25
barrysalgado: dang, yeah :/15:25
barrygary_poster: you had another topic?15:26
gary_posterheh, yeah.  one sec, lemme get the paste15:26
gary_posterI miss the old bzr review plugin a bit because it enforced our policies.15:26
gary_posterIn 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_posterI have a quick example about the ``make lint`` issue.15:26
gary_posterI 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_posterI 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_posterWe're building generic bzr tools AIUI (bzr send), but our policies are not generic, and enforcing them is valuable.15:26
gary_posterI'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_posterI also wonder if ec2test could help with the linting, but that's not as clear to me.15:26
gary_posterThe 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_posterHowever, 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_posterWe 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_posterSo: 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_posterAnd then furthermore, if you agree that we have regressed, what should we do to improve?15:27
gary_poster(done)15:27
bigjoolscan we fix lpreview to just use lpsend?15:28
gmbThere'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
gmbWell, I don't *think* there's any reason, anyway.15:28
barrygary_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-imps15:28
gary_postergmb: that's the kind of thing I'm talking about, yeah15:28
bigjoolsgmb: great minds think alike15:28
gmbabentley would know better than I.15:28
abentleygmb: Yes there is.  There's no way to specify a message body15:28
gmbabentley: That's what I was afraid you were going to say.15:29
abentleygmb: It's not that 'bzr send' couldn't, it's that it's not implemented yet.15:29
barryabentley: i wonder if there was some $EDITOR hack we could use for that15:29
gmbbarry: send just passes over to the MUA, so I'm not sure that would work.15:30
abentleybarry: If we want to force everyone to use mail_client=editor, perhaps.15:30
barryabentley: yeah, that's what i meant.  and it would just be for lp branches, so location.conf could override15:30
intellectronicaabentley: i use gmail, and so i benefit from jamesh's plugin. having to use EDITOR will make like hard for me15:30
barryabentley: not that i have any concrete ideas.  just wonderin'15:30
sinzuiI would disable such a feature because I start my cover letter when I create my branch15:31
gary_posterbarry: 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 sufficient15:31
danilosit'd be nice for lpsend to actually work with my name first, I have to manually paste the cover letter anyway15:31
barrysinzui: so do i15:31
abentleydanilos: Work with your name?15:31
BjornTdanilos: +1 :)15:31
gary_posterso the temolate thing seems like an insufficient idea15:31
marsdanilos, +1 as well :)15:32
BjornTabentley: some of us get a unicode error when using lpsend15:32
abentleyBjornT: Not from send, from Launchpad, right?15:32
danilosabentley: right15:32
BjornTabentley: right15:32
abentleySo that's not a send bug, and AIUI, work has been done to fix it this cycle.15:33
abentleyI'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
barryabentley: what are you waiting for? :)15:34
gary_posterabentley: (I don't remember hearing that before) +115:35
abentleybarry: a free moment.15:35
barryabentley: those are so overrated15:35
gary_posterok, 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
gmbabentley: Is there a bug for updating send to add that hook?15:35
abentleygmb: I don't know.15:36
barrygary_poster: yep. seems like time is better spend on send rather than lpreview though, so that's something15:36
marsgary_poster, a very astute observation :)15:36
gary_poster:-)15:36
barrygary_poster: will you check to see if there's a bug open on adding a hook for bzr send?15:36
gary_posterbarry: ok15: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't15:37
MootBotACTION 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't15:37
gary_posterbarry: IRT my previous topic: sorry, but I was not clear.  so can I just use _get_store for now, and move on?15:37
barrygary_poster: add a getStore() alias if possible and use that15:37
gary_poster...ok15:38
barrythanks15:38
gary_postersure, thanks everybody15:38
barry[TOPIC]  * Merge proposals and cover letters (barry)15:38
MootBotNew Topic:   * Merge proposals and cover letters (barry)15:38
barryso along the lines of gary_poster 's last topic...15:38
barryi notice that not everyone is using the standard cover letter template.  some people don't use any template15:38
barryshould we enforce that?  (updating the template if necessary)15:39
* mars hides15:39
* gary_poster hides15:39
barrynot blaming anyway, just wondering!15:39
* danilos hides behind gary15:39
gary_poster:-)15:39
gary_posterlol15:39
* BjornT doesn't know where the standard template is located15:39
barryif the template isn't useful or overkill, let's say so15:39
danilosI don't know where it is either, though15:39
abentleyI don't think we should enforce a template.  There are lots of times it doesn't make sense.15:39
barryhttps://dev.launchpad.net/QuickCoverLetterTemplate15:40
BjornTi think we should get bzr send to use the template. then people will automatically use it, unless they explicitly choose not to15:40
barrywell, the diff output could be removed now15:40
marsBjornT, +115:40
barryabentley: i think the template usually makes sense though15:40
marsa new plugin for bzr send that adds a --template option?15:40
gary_posterI must admit, I kind of like using lp TTW to request reviews15:41
gary_posterbut you know, if "we don't do that" that's ok15:41
barrygary_poster: you could still use the template :)15:41
gary_posterbut seems a bit of a shame, since we have a TTW interface15:41
marsgary_poster, that's a different problem - communicating a project's review preferences through the web API15:41
abentleybarry: I don't.  I find I'm usually writing the same thing under several headings.15:41
marsoops, nix the 'API' from that last statement15:42
gary_posterbarry: 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_posteroh sorry15:42
barryabentley: really?  maybe there's some confusion about what's intended for each section?15:42
flacostei preferred the old template15:43
flacosteand i still use it15:43
barrygary_poster: well, i just mean that you could paste the cover letter into the text area15:43
flacostei run bzr write-cover-letter15:43
abentleyWell, a bit, but it generally just gets in the way of a natural description of the change.15:43
flacosteand then :r cover.txt15:43
gmbflacoste: Me too.15:43
flacostewhen bzr send fires my editor15:43
gmbWell, for non-trivial branches, anyway.15:43
mars:r ?15:43
flacostemars: i see you aren't enlightened15:43
flacostevim powers!15:43
danilosmars: it's likely just some vi crap, ignore it :)15:43
marslol15:44
marsah15:44
barrydang, we're almost out of time.  let's drop this for now because i'd like to give mars's topic a shot15:44
barry[TOPIC]  * Using OCR for JavaScript reviews (mars)15:44
MootBotNew Topic:   * Using OCR for JavaScript reviews (mars)15:44
marsbarry, I'll be brief15:44
barrymars: thx15:44
marsHi everyone15:45
marsLast 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
marsAny 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
marsThese 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 jtv15:45
marsAlso, feel free to ask me if any questions come up during the review.15:45
marsthat's all15:45
* allenap discovers that TTW means Table-Top Wargaming15:45
gary_posterlol15:46
gary_posterThrough-the-web was how I've heard it used, and my intent :-)15:46
bigjoolsI thought it was ruder15:46
barrymars: thanks15:46
barryany comments on mars's request or should we break?15:47
sinzuiI 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_posterwrite it down someplace15:47
allenapSounds like a good request.15:47
intellectronicabarry: mention my item, since i don't think it requires much discussion :)15:47
marssinzui, fair enough15:47
barryintellectronica: ah, sorry15:47
sinzuiPS. I hate GMap too15:47
barry[TOPIC]  * Reviewers to enforce bug<->branch linking for all non-trivial branches (intellectronica)15:47
MootBotNew Topic:   * Reviewers to enforce bug<->branch linking for all non-trivial branches (intellectronica)15:47
marssinzui, I would love it if you could write a ranting email about that statement though15:47
barryintellectronica: +1 :)15:48
gary_posterintellectronica: (1) I assume up mean --fixes=lp:XXX?15:48
gary_poster(2) how do we check that?15:48
barrygary_poster: or you can do it TTW :)15:48
intellectronicagary_poster: either that, or you can do it ttw15:48
gary_posterah ok15:48
intellectronicaheh15:48
barryheh15:48
gary_poster:-)15:48
sinzuimars: I was going to bring it up in the meeting, but my thoughts are not collected.15:49
intellectronicaand you can check it ttw, or do it after the fact15:49
marsgary_poster, require a hook in pqm-submit, --no-fixes has to be explicitly set for this branch?15:49
marskinda like explicitly setting --no-lint15:49
barryokay, so apologies for going over.  thanks everybody and have a good day15:49
intellectronicamars: the thing is, --fixes is on commit, not on pqm submit15:49
intellectronicathanks barry15:49
barry#ENDMEETING15:49
MootBotMeeting finished at 09:49.15:49
abentleyintellectronica: Not every branch related to a bug fixes that bug.  Some are incremental branches.15:50
intellectronicaabentley: i agree. but they can still be linked to the bug15:51
abentleyintellectronica: They can't be --fixes.15:51
marssinzui, ok, but do let me know when you have something put together.  I'd love to read it.15:51
intellectronicaabentley: right, but they can be linked ttw with a comment explaining what is their relation to the bug15:51
sinzuimars I certain will15: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!