← Back to team overview

launchpad-dev team mailing list archive

ReviewerMeeting20100106

 

= ReviewerMeeting20100106 =

== summary ==

 * Check to see if there was a discussion on the mailing list about inviting other teams to do lazr-js code reviews.  If it wasn't discussed we'll address it next week. [bac/mars]
 * Reviewers with JS skills are listed on [[https://dev.launchpad.net/ReviewerSchedule]].  These people should be considered resources though any reviewer can do JS reviews.
 * Edwin suggested a new standard for JS naming.  He is going to open bugs for the existing inconsistencies and each team will do their own clean up.
 * Developers are urged to land or change the status on approved MPs that appear on +activereviews.
 * For LP developers who are not reviewers the team leads will be responsible for nominating them to become mentats when appropriate.
 * The reviewers meeting will continue on a weekly basis but may be canceled if there are no agenda items.  So please add your items to the wiki if you have them.
 * Julian brought up the recent problem we had with an API that passed review but shouldn't have.  We agreed to take the conversation to the mailing list.

== logs ==

=== ameu ===

{{{
[15:01] <bac> #startmeeting
[15:01] <MootBot> Meeting started at 09:01. The chair is bac.
[15:01] <MootBot> Commands Available: [TOPIC], [IDEA], [ACTION], [AGREED], [LINK], [VOTE]
[15:01] <bac> hello everyone.  welcome to the first reviewer's meeting of 2010.
[15:01] <bac> who's here?
[15:01] <abentley> me
[15:01] <gmb> me
[15:01] <EdwinGrubbs> me
[15:01] <sinzui> me
[15:01] <barry> me
[15:02] <henninge> me
[15:02] <noodles775> me
[15:02] <bigjools> me
[15:03] <bac> let me try to round up some folks
[15:03]  * gmb has summond the Bugs team reviewers
[15:03] <allenap> me
[15:04] <adeuring> me
[15:04] <gmb> ... see?
[15:04] <intellectronica> me
[15:04] <bac> gmb: and your leader?
[15:04] <gmb> bac: Has been pung.
[15:04] <bac> BjornT: ping
[15:04] <henninge> bac: jtv has been having connectivity troubles and danilo is off.
[15:05] <bac> thanks henninge
[15:05] <bac> well, let's get started.  if you notice an absence from one of your team members please follow gmb's good example and harass them.
[15:06] <bac> [TOPIC]  agenda
[15:06] <MootBot> New Topic:   agenda
[15:06] <bac> * Roll call
[15:06] <bac> * Action items
[15:06] <bac> * invite other teams to do lazr-js code reviews? [mars/barry]
[15:06] <bac> * Peanut gallery (anything not on the agenda)
[15:06] <bac> * Reminder: Who can do JS reviews? All reviewers? [henninge,allenap]
[15:06] <bac> * Proposed coding standard for YUI modules. [Edwin]
[15:06] <bac> * Cleaning up outstanding approved branches on +activereviews [bac]
[15:06] <bac> * New developers as mentats? [bac]
[15:06] <bac> * Meeting frequency [bac]
[15:06] <abentley> bac: I am the sole member of the Code team on this meeting, but I welcome harassment from others.
[15:06] <bac> abentley: what about rockstar?
[15:06] <abentley> bac: He does the other one.
[15:07] <bac> [TOPIC] action items
[15:07] <MootBot> New Topic:  action items
[15:07] <barry> rockstar joins ameu
[15:07] <bac> first, an unlisted item -- we all owe many thanks to barry for his long service getting the group together and chairing.  thanks barry and have fun in foundations!
[15:07] <henninge> yeah, thanks barry!
[15:08] <intellectronica> ees a jolly good fella
[15:08] <abentley> barry: Thanks!
[15:08] <bac> [TOPIC] * invite other teams to do lazr-js code reviews? [mars/barry]
[15:08] <MootBot> New Topic:  * invite other teams to do lazr-js code reviews? [mars/barry]
[15:08]  * barry blushes - you're welcome!  i have no doubt bac will great improve the governance of this team :)
[15:08] <al-maisan> me
[15:08] <bac> i'm not sure if this is leftover from our last meeting so long ago
[15:09] <bac> mars isn't here, so do you recall barry?
[15:09] <barry> i vaguely remember an ml discussion about this from way back last year
[15:09] <barry> i think it would be a good idea to do cross-team reviews of lazr-js, but iirc mars was -0 on it
[15:10] <barry> i don't remember why (something about the code not being ready yet?)
[15:10] <bac> ok, i'll take it on to review the ML to see if i can find a discussion and talk to mars to see if we want to pursue it.
[15:10] <bac> [TOPIC] * Reminder: Who can do JS reviews? All reviewers? [henninge,allenap]
[15:10] <MootBot> New Topic:  * Reminder: Who can do JS reviews? All reviewers? [henninge,allenap]
[15:11] <bac> henninge: is this a current issue?  if so, please proceed.
[15:11] <henninge> bac: it came up in a review
[15:12] <henninge> I think allenap wasn't sure if he could review my JS code because he was never officially knighted as "JS reviewer"
[15:12] <barry> all reviewers can and should do js reviewes
[15:12] <barry> *ui* reviews are a different matter
[15:12] <henninge> barry: that's what I remember, thanks.
[15:12] <fjlacoste> well
[15:12] <henninge> yup
[15:12] <intellectronica> i agree, by now there should be no reason why anyone can't do js reviews
[15:12] <bac> that's my feeling.  though i know i've seen some people who don't consider them experts defer.  i've done it myself.
[15:13] <fjlacoste> we did have a similar JS review approval process
[15:13] <fjlacoste> graduated reviewers were the UI/AJAX team members
[15:13] <fjlacoste> that attended the Berlin sprint
[15:13] <intellectronica> and if someone doesn't feel comfortable enough then they can work with someone else
[15:13] <barry> bac: right.  that's not to say a reviewer can't ask for help, with js or even python
[15:13] <fjlacoste> where JS coding guidlines were established
[15:13] <fjlacoste> but we never graduated anybody after that
[15:13] <fjlacoste> and didn't make the process very formal either
[15:14] <barry> fjlacoste: i'm nearly certain we decided to throw everyone in the deep end :)
[15:14] <bac> fjlacoste: perhaps we consider those people as resources but everyone should attemp to do JS reviews to their comfort level
[15:14] <fjlacoste> right
[15:14] <fjlacoste> everyone was considered a mentee
[15:14] <fjlacoste> well
[15:14] <fjlacoste> we didn't setup a formal mentoring process around this
[15:14] <fjlacoste> we shuld clarify that situation
[15:15] <fjlacoste> and update the reviewer pages
[15:15] <fjlacoste> accordingly
[15:15] <barry> +1 on updating the reviewer page
[15:15] <bac> perhaps we need a volunteer to herd the JS reviewers.  anyone?
[15:16] <henninge> I thought we didn't have such a group?
=== fjlacoste is now known as flacoste
[15:17] <bac> henninge: i don't recall
[15:17] <flacoste> bac: i think this should be someone from the UI/AJAX team
[15:18] <bac> flacoste: agreed.
[15:18] <bac> EdwinGrubbs: would you be interested?
[15:18] <henninge> bac: What I meant is, if all reviewers are JS reviewers, there is no such special group, is there?
[15:19] <EdwinGrubbs> bac: to herd js reviewers? don't we have a list of them already in the wiki.
[15:19] <noodles775> And it already says: https://dev.launchpad.net/ReviewerSchedule
[15:19] <noodles775> A Note on JavaScript reviews: Any reviewer can handle a JavaScript review, if they feel comfortable doing so. For now, we ask that their review by seconded by one of the JavaScript specialists.
[15:20] <bac> thanks noodles775
[15:20] <bac> it looks like there is no action necessary.
[15:20] <bac> let's move on.
[15:20] <bac> [TOPIC] * Proposed coding standard for YUI modules. [Edwin]
[15:20] <MootBot> New Topic:  * Proposed coding standard for YUI modules. [Edwin]
[15:21] <abentley> noodles775: So this means there is a "JavaScript specialists" group.  Do we have an easy way to find its members?
[15:21]  * henninge waits for the clarification on the wiki ... ;-)
[15:21] <noodles775> abentley: the list on that page identifies them I think...
[15:21] <bac> abentley: that page lists javascript reviewers in the last column
[15:21] <noodles775> (you can update yourself as a resource of course)
[15:22] <abentley> noodles775, bac: sounds fine.
[15:22] <EdwinGrubbs> you may also want to look at the inconsistencies we currently have with JS module names and the namespaces they define: https://pastebin.canonical.com/25818/
[15:23] <noodles775> That looks like a good topic in itself :)
[15:23] <bac> EdwinGrubbs: the floor is yours for your YUI topic.
[15:23] <sinzui> I knew milestone_table would bite me
[15:23] <EdwinGrubbs> I'm suggesting that we name our JS modules more like how python modules must be named. More info is available at https://dev.launchpad.net/ReviewerMeetingAgenda   but I"ll summarize
[15:24] <EdwinGrubbs> 1. The module name should match the directory structure. E.g. javascript/registry/timeline.js should use YUI().add('registry.timeline', ...
[15:25] <EdwinGrubbs> 2. The namespace should match the module name, so we should put methods in the namespace like this Y.registry.timeline.someFunction() instead of Y.registry.someFunction().
[15:26] <EdwinGrubbs> does anybody disagree with that plan?
[15:27] <noodles775> Not me - it would be good to not have to think about those decisions :)
[15:27] <intellectronica> +1
[15:27] <sinzui> your next question should be who volunteers to fix these
[15:27] <barry> +1
[15:27] <abentley> +1
[15:27] <deryck> +1
[15:27] <henninge> +1
[15:28] <bac> so we seem to agree it's a good idea.  which leads to curtis' question of who and when to do the clean up.
[15:28] <EdwinGrubbs> I can open up bugs for the inconsistent modules and assign them to the respective teams.
[15:28] <intellectronica> since the code is already divided by app, each team can take care of their own
[15:29] <henninge> EdwinGrubbs: that would be great.
[15:29] <bac> thanks Edwin
[15:30] <bac> [ACTION] Edwin to file bugs on JS naming inconsistencies and teams will take care of doing the clean up.
[15:30] <MootBot> ACTION received:  Edwin to file bugs on JS naming inconsistencies and teams will take care of doing the clean up.
[15:30] <bac> [TOPIC] * Cleaning up outstanding approved branches on +activereviews [bac]
[15:30] <MootBot> New Topic:  * Cleaning up outstanding approved branches on +activereviews [bac]
[15:31] <bac> i noticed yesterday when doing OCR that we've got a large number of approved branches that haven't landed.
[15:32] <bac> is this work abandoned after review, blocked, other?
[15:32] <abentley> bac: Mine were blocked on test suite issues, but are now moving again.
[15:32] <bac> if the former perhaps the state of the MP can changed to reflect it and clear out that list.
[15:34] <intellectronica> bac: maybe each reviewer at the start/end of their shift try and chase those MPs in question
[15:34] <bac> i'm glad that tim created the list and think we should strive to keep it minimal.  any other thoughts?
[15:34] <intellectronica> it's a bit of a bother, but it will probably help and is not hard to do
[15:34] <abentley> bac: For the first case, they can be marked "rejected" or "work-in-progress", as appropriate.
[15:35] <bac> abentley: right.
[15:35] <abentley> e.g. jelmer's branch was approved, but it turns out there are some issues that require further investigation.
[15:36] <bac> intellectronica: perhaps.  or we might just monitor it weekly, perhaps keeping it as an item for this meeting until the backlog is handled
[15:36] <bac> for now, i just ask that each developer look at his branches and take the necessary action.
[15:37] <bac> [TOPIC] * New developers as mentats? [bac]
[15:37] <MootBot> New Topic:  * New developers as mentats? [bac]
[15:38] <bac> we've hired a few new people and i was wondering if anyone was ready to enter the reviewer mentat program.
[15:38] <bac> i think team leads should be responsible for nominating their developers as appropriate
[15:39] <bac> moving on to the final topic
[15:39] <bac> [TOPIC] * Meeting frequency [bac]
[15:39] <MootBot> New Topic:  * Meeting frequency [bac]
[15:40] <bac> in the past we have met weekly.  starting the new year i think we should look at whether we want to continue weekly meetings or move to biweekly.
[15:40]  * bigjools agrees with bac
[15:41] <abentley> I would prefer to continue meeting weekly, because it's easier to remember.
[15:41] <bigjools> I say stick with weekly, if there's nothing to discuss it's not a problem is it?  We just finish quickly.
[15:41] <intellectronica> i think bi-weekly will be enough. maybe we can alternate the eu and pacific meetings, so that if you really want to join a meeting on a given week you can have the option of joining out of office hours
[15:41] <intellectronica> bigjools: there is a bit of overhead to a meeting
[15:42] <bac> bigjools: alternatively, if there are no/few items on the agenda i can pre-emptively cancel the meeting
[15:42] <bigjools> not a metric!
[15:42] <bac> bigjools: but, as a rule it would go on as planned on a weekly basis
[15:43] <gmb> I'm +0 on keeping it weekly for the sake of my godawful memory.
[15:43] <bigjools> I don't see the problem personally
[15:43] <bac> i just don't want to cause interruption to everyone's schedule if the meeting is not serving a purpose
[15:43] <intellectronica> also, many of the topics we discuss in these meetings can probably be discussed more productively on the mailing list anyway
[15:44] <gmb> bac: Well, if it's going to break up an important piece of work we can always send our apologies...
[15:44] <bac> ok, it sounds like there is enough sentiment to continue weekly.
[15:44] <bac> lastly
[15:44] <bac> [TOPIC] Peanut gallery
[15:44] <MootBot> New Topic:  Peanut gallery
[15:45] <bac> anyone have an item they'd like to (briefly) discuss?
[15:45]  * bigjools raises hand
[15:45] <bac> go bigjools
[15:45]  * abentley raises hand
[15:45] <bac> abentley on deck
[15:45] <bigjools> very quickly, the current edge non-updating is because we let an API change land which didn't have security protection
[15:46] <bigjools> so this is a reminder to be vigilant when reviewing API changes
[15:46]  * bigjools out
[15:46] <abentley> bigjools: Thanks for raising my topic
[15:46] <bigjools> heh
[15:46] <bac> thanks bigjools and abentley
[15:46] <abentley> I wanted to ask if we think there is anything else we should do.
[15:47] <abentley> The outcome of the discussion was that this should have been caught in review.
[15:47] <bigjools> reviewers' checklists?
[15:47] <abentley> bigjools: Wouldn't hurt.
[15:47] <bigjools> maybe a template for review replies. Didn't we used to have one? :)
[15:47] <abentley> bigjools: You mean for review requests?
[15:47] <bigjools> no, replies
[15:48] <bigjools> I remember using barry's reviewing tool
[15:48] <abentley> bigjools: Must have been before my time.
[15:48] <abentley> Of course, we've exposed a lot of APIs, and if this is the only security issue we've had, we're doing pretty well.
[15:48] <abentley> But what if it's not?
[15:49] <bigjools> it's not the first time it's happened
[15:49] <abentley> Would it be a good idea to audit our API?
[15:49] <bigjools> I think security should be constantly on reviewers' minds
[15:50] <bigjools> we can continue this on the list
[15:50] <bigjools> we're OOT
[15:50] <bac> yep
[15:50] <abentley> bigjools: Okay.
[15:50] <bac> thanks for coming and contributing
[15:50] <bac> #endmeeting
[15:50] <MootBot> Meeting finished at 09:50.
[15:51] <henninge> Thanks bac!
}}}

=== asiapac ===

Very brief meeting that just summarized the AMEU meeting.  No new discussion.