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.