Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

Code review thoughts: module reviewers

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


bvibber at wikimedia

Dec 20, 2011, 3:01 PM

Post #1 of 8 (411 views)
Permalink
Code review thoughts: module reviewers

We've sorta informally done things were revisions have gotten tagged for
some particular person's review as an area of specialty, but we've never
really had formal division of labor among separate parts -- nothing for
instance that can be used to automatically queue things up for particular
peoples' inboxes for timely review.

Often, little things are suitable for many people to look at, but major
subsystem refactorings -- like the landing of Aaron's file backend changes
-- really are specialized and need to be looked over by somebody who's a
specialist, rather than just whoever gets around to looking it over.

I'd like us to seriously consider having primary reviewers for various code
modules, so things like this get handled asap and don't end up falling
through the cracks -- big changes, and small confusing changes ;) -- should
get pretty consistently treated.


Projects like Firefox or the Linux kernel tend to have responsible parties
for various modules, who either manage ingestion of patches through the
source control or issue tracker and do testing, review, feedback, and
eventual merging. I think we would do well to emulate this a little more
explicitly than we do today, especially if we're trying to keep from having
an ever-growing review backlog.

Thoughts? Volunteers?

-- brion
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Platonides at gmail

Dec 20, 2011, 4:25 PM

Post #2 of 8 (405 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

Other than the temptation of assigning everything to you? ;)

Yes, I like the proposal. Although I would make a more complex setup,
separating it on:

* Knowledge areas a revision may be touching (identified by a regex)
* People with a level on each area
* Revision scoring (so instead of passing without reviewing, you can tag
it with the expertise level you guess is suitable for that revision)
* Notifications for users/area/level (opting-in for revisions of level >
0 to an area, mean being notificated of every change)
* Special page listing pending changes by area, sorted by complexity.



_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


robla at wikimedia

Dec 20, 2011, 10:46 PM

Post #3 of 8 (398 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

On Tue, Dec 20, 2011 at 3:01 PM, Brion Vibber <bvibber [at] wikimedia> wrote:
> I'd like us to seriously consider having primary reviewers for various code
> modules, so things like this get handled asap and don't end up falling
> through the cracks -- big changes, and small confusing changes ;) -- should
> get pretty consistently treated.

I think this is a grand idea. A few code areas:

Parser (3 revs)
https://www.mediawiki.org/w/index.php?path=%2Ftrunk%2Fphase3%2Fincludes%2Fparser&title=Special%3ACode%2FMediaWiki%2Fstatus%2Fnew

FIlerepo (12 revs)
https://www.mediawiki.org/w/index.php?path=%2Ftrunk%2Fphase3%2Fincludes%2Ffilerepo&title=Special%3ACode%2FMediaWiki%2Fstatus%2Fnew

API (4 revs)
https://www.mediawiki.org/w/index.php?path=%2Ftrunk%2Fphase3%2Fincludes%2Fapi&title=Special%3ACode%2FMediaWiki%2Fstatus%2Fnew

Resource Loader (4 revs):
https://www.mediawiki.org/w/index.php?path=%2Ftrunk%2Fphase3%2Fincludes%2Fresourceloader&title=Special%3ACode%2FMediaWiki%2Fstatus%2Fnew

Someone will need to do some more looking at different ways of slicing
things up. So far, I'm not coming up with many easy ways of dividing
core with this style of query. Extensions should be a lot easier to
divvy up. Anyone got good ideas for how to make this work?

Rob

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


roan.kattouw at gmail

Dec 21, 2011, 12:44 AM

Post #4 of 8 (398 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

On Wed, Dec 21, 2011 at 12:01 AM, Brion Vibber <bvibber [at] wikimedia> wrote:
> Often, little things are suitable for many people to look at, but major
> subsystem refactorings -- like the landing of Aaron's file backend changes
> -- really are specialized and need to be looked over by somebody who's a
> specialist, rather than just whoever gets around to looking it over.
>
> I'd like us to seriously consider having primary reviewers for various code
> modules, so things like this get handled asap and don't end up falling
> through the cracks -- big changes, and small confusing changes ;) -- should
> get pretty consistently treated.
>
YES. I think I've proposed something like this before (a year ago?),
but the people I talked to thought it would be unnecessary bureaucracy
IIRC. I guess the CR situation has gotten sufficiently worse that
we're finally considering this.

How about we set up a wiki page where we list who's an expert in what
area (documenting this would be useful in its own right BTW), so we
can figure out who'll be the primary reviewer for what, and put that
on the wiki page too?

Roan

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


jpostlethwaite at wikimedia

Dec 21, 2011, 10:25 AM

Post #5 of 8 (397 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

It might be helpful to list a primary and a few secondaries on some areas.

On Wed, Dec 21, 2011 at 12:44 AM, Roan Kattouw <roan.kattouw [at] gmail>wrote:

> On Wed, Dec 21, 2011 at 12:01 AM, Brion Vibber <bvibber [at] wikimedia>
> wrote:
> > Often, little things are suitable for many people to look at, but major
> > subsystem refactorings -- like the landing of Aaron's file backend
> changes
> > -- really are specialized and need to be looked over by somebody who's a
> > specialist, rather than just whoever gets around to looking it over.
> >
> > I'd like us to seriously consider having primary reviewers for various
> code
> > modules, so things like this get handled asap and don't end up falling
> > through the cracks -- big changes, and small confusing changes ;) --
> should
> > get pretty consistently treated.
> >
> YES. I think I've proposed something like this before (a year ago?),
> but the people I talked to thought it would be unnecessary bureaucracy
> IIRC. I guess the CR situation has gotten sufficiently worse that
> we're finally considering this.
>
> How about we set up a wiki page where we list who's an expert in what
> area (documenting this would be useful in its own right BTW), so we
> can figure out who'll be the primary reviewer for what, and put that
> on the wiki page too?
>
> Roan
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>



--
Jeremy Postlethwaite
jpostlethwaite [at] wikimedia
515-839-6885 x6790
Backend Software Developer
Wikimedia Foundation <http://wikimediafoundation.org/>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


neilk at wikimedia

Dec 21, 2011, 12:03 PM

Post #6 of 8 (397 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

Definitely a good idea.

I'd like to further suggest that reviewers for particular parts of the
codebase ought to be listed somewhere publicly. Rightly or wrongly a lot
of people want guidance before embarking on a project to fix something.
So if somebody has an idea or a patch, they don't have to do the
"anyone? anyone? Bueller?" thing in bugzilla or wikitech-l. These
reviewers could evolve into mentors, if they are so inclined.

When we have git, maybe we can even automate this with pull requests...

Of course we should still allow it to be as flexible as possible. It
should be easy to re-assign a change to someone else's queue. No one
person should be able to block commits.


On 12/20/11 3:01 PM, Brion Vibber wrote:
> We've sorta informally done things were revisions have gotten tagged for
> some particular person's review as an area of specialty, but we've never
> really had formal division of labor among separate parts -- nothing for
> instance that can be used to automatically queue things up for particular
> peoples' inboxes for timely review.
>
> Often, little things are suitable for many people to look at, but major
> subsystem refactorings -- like the landing of Aaron's file backend changes
> -- really are specialized and need to be looked over by somebody who's a
> specialist, rather than just whoever gets around to looking it over.
>
> I'd like us to seriously consider having primary reviewers for various code
> modules, so things like this get handled asap and don't end up falling
> through the cracks -- big changes, and small confusing changes ;) -- should
> get pretty consistently treated.
>
>
> Projects like Firefox or the Linux kernel tend to have responsible parties
> for various modules, who either manage ingestion of patches through the
> source control or issue tracker and do testing, review, feedback, and
> eventual merging. I think we would do well to emulate this a little more
> explicitly than we do today, especially if we're trying to keep from having
> an ever-growing review backlog.
>
> Thoughts? Volunteers?
>
> -- brion
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l

--
Neil Kandalgaonkar ( <neilk [at] wikimedia>

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Dec 21, 2011, 1:02 PM

Post #7 of 8 (398 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

On Wed, Dec 21, 2011 at 3:03 PM, Neil Kandalgaonkar <neilk [at] wikimedia> wrote:
> When we have git, maybe we can even automate this with pull requests...
>

Minor nitpick--we won't have pull requests. It's push-for-review.

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


robla at wikimedia

Dec 23, 2011, 12:12 AM

Post #8 of 8 (392 views)
Permalink
Re: Code review thoughts: module reviewers [In reply to]

On Tue, Dec 20, 2011 at 3:01 PM, Brion Vibber <bvibber [at] wikimedia> wrote:
> I'd like us to seriously consider having primary reviewers for various code
> modules, so things like this get handled asap and don't end up falling
> through the cracks -- big changes, and small confusing changes ;) -- should
> get pretty consistently treated.

Mark and I have started the process of tagging code areas, which you
can see reflected on the revision report:
http://www.mediawiki.org/wiki/MediaWiki_roadmap/1.19/Revision_report

There are still a lot of revisions that are still untagged, but not
nearly as many as there were a day ago. The tag names are mainly
based on extensions right now, but there is a small amount of grouping
going on. If someone wants to come up with a more sensible tagging
structure, by all means go for it.

I hope this is a helpful first step. After the holidays, we can get
serious about dividing up the work based on tag.

Rob

_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.