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

Mailing List Archive: Wikipedia: Wikitech

CodeReview auto-deferral regexes

 

 

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


roan.kattouw at gmail

Jul 20, 2010, 4:38 AM

Post #1 of 23 (2579 views)
Permalink
CodeReview auto-deferral regexes

Auto-deferral regexes for CodeReview were implemented in r63277 [1],
and I deployed this feature three weeks ago. It allows us to set an
array of regexes that will be matched against the path of each new
commit; if one of them matches, the commit is automatically marked as
'deferred' instead of 'new'.

There are a few limitations to this implementation that are important
to understand:
* it only matches paths, not commit summaries. This means
auto-deferring e.g. TranslateWiki exports is harder
* it only matches the root path of the commit, which is very often an
uninformative one like /trunk/phase3 , /trunk/extensions , /trunk or
even / . This means you can't e.g. auto-defer all commits touching a
certain file or path

Despite these limitations, this feature could be quite useful for
autodeferring at least some large parts of the repository we don't
care about review-wise. Any suggestions for paths to auto-defer?

Roan Kattouw (Catrope)

[1] http://www.mediawiki.org/wiki/Special:Code/MediaWiki/63277

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


daniel at brightbyte

Jul 20, 2010, 4:46 AM

Post #2 of 23 (2538 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Roan Kattouw schrieb:
> Despite these limitations, this feature could be quite useful for
> autodeferring at least some large parts of the repository we don't
> care about review-wise. Any suggestions for paths to auto-defer?

http://svn.wikimedia.org/svnroot/mediawiki/trunk/WikiWord/ is basically my
personal pet project. If you can keep it out of the review queue, that would
probably be an improvement for everyone :)

-- daniel

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


innocentkiller at gmail

Jul 20, 2010, 6:10 AM

Post #3 of 23 (2538 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Tue, Jul 20, 2010 at 7:46 AM, Daniel Kinzler <daniel [at] brightbyte> wrote:
> Roan Kattouw schrieb:
>> Despite these limitations, this feature could be quite useful for
>> autodeferring at least some large parts of the repository we don't
>> care about review-wise. Any suggestions for paths to auto-defer?
>
> http://svn.wikimedia.org/svnroot/mediawiki/trunk/WikiWord/ is basically my
> personal pet project. If you can keep it out of the review queue, that would
> probably be an improvement for everyone :)
>
> -- daniel
>

I was going to suggest that. And might I be so bold as to suggest
the Semantic* extensions. They're generally marked as deferred
without review.

I also want to make a quick disclaimer. This doesn't mean that
these code paths don't matter. It just means they aren't actively
reviewed on CR, so we want to just automate what reviewers are
doing manually anyway

-Chad

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


jeroendedauw at gmail

Jul 20, 2010, 6:16 AM

Post #4 of 23 (2535 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Hey,

About the semantic extensions:

It would actually be nice if they did not get marked deferred at all, and be
reviewed by people that are familiar with them to some extend. I'm willing
to do that for all commits not made by myself. Assuming this would not
interfere to much with WMF code review of course :)

Cheers

--
Jeroen De Dauw
* http://blog.bn2vs.com
* http://wiki.bn2vs.com
Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
66 65!
--


On 20 July 2010 15:10, Chad <innocentkiller [at] gmail> wrote:

> On Tue, Jul 20, 2010 at 7:46 AM, Daniel Kinzler <daniel [at] brightbyte>
> wrote:
> > Roan Kattouw schrieb:
> >> Despite these limitations, this feature could be quite useful for
> >> autodeferring at least some large parts of the repository we don't
> >> care about review-wise. Any suggestions for paths to auto-defer?
> >
> > http://svn.wikimedia.org/svnroot/mediawiki/trunk/WikiWord/ is basically
> my
> > personal pet project. If you can keep it out of the review queue, that
> would
> > probably be an improvement for everyone :)
> >
> > -- daniel
> >
>
> I was going to suggest that. And might I be so bold as to suggest
> the Semantic* extensions. They're generally marked as deferred
> without review.
>
> I also want to make a quick disclaimer. This doesn't mean that
> these code paths don't matter. It just means they aren't actively
> reviewed on CR, so we want to just automate what reviewers are
> doing manually anyway
>
> -Chad
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Jul 20, 2010, 6:20 AM

Post #5 of 23 (2534 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Tue, Jul 20, 2010 at 9:16 AM, Jeroen De Dauw <jeroendedauw [at] gmail> wrote:
> Hey,
>
> About the semantic extensions:
>
> It would actually be nice if they did not get marked deferred at all, and be
> reviewed by people that are familiar with them to some extend. I'm willing
> to do that for all commits not made by myself. Assuming this would not
> interfere to much with WMF code review of course :)
>
> Cheers
>

If someone's going to start doing code review, that's fine. They've
just all been getting deferred because nobody's been reviewing
them so far.

-Chad

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


maxsem.wiki at gmail

Jul 20, 2010, 8:20 AM

Post #6 of 23 (2529 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On 20.07.2010, 17:20 Chad wrote:

> On Tue, Jul 20, 2010 at 9:16 AM, Jeroen De Dauw <jeroendedauw [at] gmail> wrote:
>> Hey,
>>
>> About the semantic extensions:
>>
>> It would actually be nice if they did not get marked deferred at all, and be
>> reviewed by people that are familiar with them to some extend. I'm willing
>> to do that for all commits not made by myself. Assuming this would not
>> interfere to much with WMF code review of course :)
>>
>> Cheers
>>

> If someone's going to start doing code review, that's fine. They've
> just all been getting deferred because nobody's been reviewing
> them so far.

We could create a separate review queue for it.


--
Best regards,
Max Semenik ([[User:MaxSem]])


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


jeroendedauw at gmail

Jul 20, 2010, 8:25 AM

Post #7 of 23 (2522 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Hey,

That would be completely awesome!

Cheers

--
Jeroen De Dauw
* http://blog.bn2vs.com
* http://wiki.bn2vs.com
Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
66 65!
--


On 20 July 2010 17:20, Max Semenik <maxsem.wiki [at] gmail> wrote:

> On 20.07.2010, 17:20 Chad wrote:
>
> > On Tue, Jul 20, 2010 at 9:16 AM, Jeroen De Dauw <jeroendedauw [at] gmail>
> wrote:
> >> Hey,
> >>
> >> About the semantic extensions:
> >>
> >> It would actually be nice if they did not get marked deferred at all,
> and be
> >> reviewed by people that are familiar with them to some extend. I'm
> willing
> >> to do that for all commits not made by myself. Assuming this would not
> >> interfere to much with WMF code review of course :)
> >>
> >> Cheers
> >>
>
> > If someone's going to start doing code review, that's fine. They've
> > just all been getting deferred because nobody's been reviewing
> > them so far.
>
> We could create a separate review queue for it.
>
>
> --
> Best regards,
> Max Semenik ([[User:MaxSem]])
>
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


markus at semantic-mediawiki

Jul 20, 2010, 10:33 AM

Post #8 of 23 (2524 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On 20/07/2010 16:25, Jeroen De Dauw wrote:
> Hey,
>
> That would be completely awesome!

Yes, that would help a lot, although it might be appropriate to still
auto-defer reviews for some extensions in this queue, depending on
whether or not enough reviewing happens for them (calling an extension
"Semantic..." does not imply that SMW developers will review it ;-).

Markus

>
> Cheers
>
> --
> Jeroen De Dauw
> * http://blog.bn2vs.com
> * http://wiki.bn2vs.com
> Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
> 66 65!
> --
>
>
> On 20 July 2010 17:20, Max Semenik<maxsem.wiki [at] gmail> wrote:
>
>> On 20.07.2010, 17:20 Chad wrote:
>>
>>> On Tue, Jul 20, 2010 at 9:16 AM, Jeroen De Dauw<jeroendedauw [at] gmail>
>> wrote:
>>>> Hey,
>>>>
>>>> About the semantic extensions:
>>>>
>>>> It would actually be nice if they did not get marked deferred at all,
>> and be
>>>> reviewed by people that are familiar with them to some extend. I'm
>> willing
>>>> to do that for all commits not made by myself. Assuming this would not
>>>> interfere to much with WMF code review of course :)
>>>>
>>>> Cheers
>>>>
>>
>>> If someone's going to start doing code review, that's fine. They've
>>> just all been getting deferred because nobody's been reviewing
>>> them so far.
>>
>> We could create a separate review queue for it.
>>
>>
>> --
>> Best regards,
>> Max Semenik ([[User:MaxSem]])
>>
>>
>> _______________________________________________
>> Wikitech-l mailing list
>> Wikitech-l [at] lists
>> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>


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


Platonides at gmail

Jul 20, 2010, 11:01 AM

Post #9 of 23 (2535 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Roan Kattouw wrote:
> Auto-deferral regexes for CodeReview were implemented in r63277 [1],
> and I deployed this feature three weeks ago. It allows us to set an
> array of regexes that will be matched against the path of each new
> commit; if one of them matches, the commit is automatically marked as
> 'deferred' instead of 'new'.

It should probably allow different status.


> There are a few limitations to this implementation that are important
> to understand:
> * it only matches paths, not commit summaries. This means
> auto-deferring e.g. TranslateWiki exports is harder
> * it only matches the root path of the commit, which is very often an
> uninformative one like /trunk/phase3 , /trunk/extensions , /trunk or
> even / . This means you can't e.g. auto-defer all commits touching a
> certain file or path

Why not base it also in $rev->mPaths ?
We don't touch so many files for it to be problematic on server
resources, and an auto-defer for commits with all files matching
"(^/trunk/phase3/languages/messages/|.i18n.php$)" would nicely skip all
translatewiki updates.

Although a hook entry may be a more appropiate way.


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


jeroendedauw at gmail

Jul 21, 2010, 1:04 AM

Post #10 of 23 (2516 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Hey,

Is someone planning on doing this? If not, who can do it? The sooner it's
there, the better.

Cheers

--
Jeroen De Dauw
* http://blog.bn2vs.com
* http://wiki.bn2vs.com
Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
66 65!
--


On 20 July 2010 17:20, Max Semenik <maxsem.wiki [at] gmail> wrote:

> On 20.07.2010, 17:20 Chad wrote:
>
> > On Tue, Jul 20, 2010 at 9:16 AM, Jeroen De Dauw <jeroendedauw [at] gmail>
> wrote:
> >> Hey,
> >>
> >> About the semantic extensions:
> >>
> >> It would actually be nice if they did not get marked deferred at all,
> and be
> >> reviewed by people that are familiar with them to some extend. I'm
> willing
> >> to do that for all commits not made by myself. Assuming this would not
> >> interfere to much with WMF code review of course :)
> >>
> >> Cheers
> >>
>
> > If someone's going to start doing code review, that's fine. They've
> > just all been getting deferred because nobody's been reviewing
> > them so far.
>
> We could create a separate review queue for it.
>
>
> --
> Best regards,
> Max Semenik ([[User:MaxSem]])
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Jul 21, 2010, 4:58 AM

Post #11 of 23 (2515 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Wed, Jul 21, 2010 at 4:04 AM, Jeroen De Dauw <jeroendedauw [at] gmail> wrote:
> Hey,
>
> Is someone planning on doing this? If not, who can do it? The sooner it's
> there, the better.
>
> Cheers
>
> --
> Jeroen De Dauw
> * http://blog.bn2vs.com
> * http://wiki.bn2vs.com
> Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
> 66 65!
> --
>

I fixed one issue in r69675. $wgCodeReviewDeferredPaths applied to all
repositories.
So if we had a separate queue for Semantic things, it would have
applied the main
repo's deferral regexes. This has been fixed.

I'm also not sure how Code Review will handle a repository handling a
subset of another
repository. I'm pretty sure things will be ok, I only imagine it would
just duplicate data
(revs for SMW stuff would be imported for both repos). Still should be
tested first though.
Then we would need someone with repoadmin rights to set this up, I
believe Brion or
Tim can.

-Chad

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


roan.kattouw at gmail

Jul 21, 2010, 5:38 AM

Post #12 of 23 (2517 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

2010/7/21 Chad <innocentkiller [at] gmail>:
> I'm also not sure how Code Review will handle a repository handling a
> subset of another
> repository. I'm pretty sure things will be ok, I only imagine it would
> just duplicate data
> (revs for SMW stuff would be imported for both repos). Still should be
> tested first though.
> Then we would need someone with repoadmin rights to set this up, I
> believe Brion or
> Tim can.
>
Why would you want to do this? With the path search feature, it's
extremely easy to pull up a list of revs touching a certain extension.
I really don't see why the SMW review queue has to be separate from
the main MW review queue on a technical level; of course it would be
on a personal level, in that different people review different things,
but we have that already for e.g. UsabilityInitiative. In practical
terms, people who are familiar with the SMW codebase would start
reviewing SMW revisions through our existing CodeReview setup, and the
only thing we would have to do on a technical level is make sure those
paths don't get auto-deferred.

Roan Kattouw (Catrope)

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


innocentkiller at gmail

Jul 21, 2010, 5:47 AM

Post #13 of 23 (2514 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Wed, Jul 21, 2010 at 8:38 AM, Roan Kattouw <roan.kattouw [at] gmail> wrote:
> 2010/7/21 Chad <innocentkiller [at] gmail>:
>> I'm also not sure how Code Review will handle a repository handling a
>> subset of another
>> repository. I'm pretty sure things will be ok, I only imagine it would
>> just duplicate data
>> (revs for SMW stuff would be imported for both repos). Still should be
>> tested first though.
>> Then we would need someone with repoadmin rights to set this up, I
>> believe Brion or
>> Tim can.
>>
> Why would you want to do this? With the path search feature, it's
> extremely easy to pull up a list of revs touching a certain extension.
> I really don't see why the SMW review queue has to be separate from
> the main MW review queue on a technical level; of course it would be
> on a personal level, in that different people review different things,
> but we have that already for e.g. UsabilityInitiative. In practical
> terms, people who are familiar with the SMW codebase would start
> reviewing SMW revisions through our existing CodeReview setup, and the
> only thing we would have to do on a technical level is make sure those
> paths don't get auto-deferred.
>
> Roan Kattouw (Catrope)
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>

I agree with you here. They were just suggesting another route.
Honestly, I don't really care either way :) The fix in r69675 is
generally useful though, if repositories were segemented in that
manner.

-Chad

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


roan.kattouw at gmail

Jul 21, 2010, 5:50 AM

Post #14 of 23 (2518 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

2010/7/21 Chad <innocentkiller [at] gmail>:
> I agree with you here. They were just suggesting another route.
> Honestly, I don't really care either way :) The fix in r69675 is
> generally useful though, if repositories were segemented in that
> manner.
>
...or if you happen to have repositories containing identical or similar paths.

Roan Kattouw (Catrope)

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


jeroendedauw at gmail

Jul 21, 2010, 5:54 AM

Post #15 of 23 (2517 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Hey,

I'm also fine either way. So if no separate queue is set up, I'd appropriate
it if the semantic* commits where not marked as deferred from now on.

Cheers

--
Jeroen De Dauw
* http://blog.bn2vs.com
* http://wiki.bn2vs.com
Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
66 65!
--


On 21 July 2010 14:47, Chad <innocentkiller [at] gmail> wrote:

> On Wed, Jul 21, 2010 at 8:38 AM, Roan Kattouw <roan.kattouw [at] gmail>
> wrote:
> > 2010/7/21 Chad <innocentkiller [at] gmail>:
> >> I'm also not sure how Code Review will handle a repository handling a
> >> subset of another
> >> repository. I'm pretty sure things will be ok, I only imagine it would
> >> just duplicate data
> >> (revs for SMW stuff would be imported for both repos). Still should be
> >> tested first though.
> >> Then we would need someone with repoadmin rights to set this up, I
> >> believe Brion or
> >> Tim can.
> >>
> > Why would you want to do this? With the path search feature, it's
> > extremely easy to pull up a list of revs touching a certain extension.
> > I really don't see why the SMW review queue has to be separate from
> > the main MW review queue on a technical level; of course it would be
> > on a personal level, in that different people review different things,
> > but we have that already for e.g. UsabilityInitiative. In practical
> > terms, people who are familiar with the SMW codebase would start
> > reviewing SMW revisions through our existing CodeReview setup, and the
> > only thing we would have to do on a technical level is make sure those
> > paths don't get auto-deferred.
> >
> > Roan Kattouw (Catrope)
> >
> > _______________________________________________
> > Wikitech-l mailing list
> > Wikitech-l [at] lists
> > https://lists.wikimedia.org/mailman/listinfo/wikitech-l
> >
>
> I agree with you here. They were just suggesting another route.
> Honestly, I don't really care either way :) The fix in r69675 is
> generally useful though, if repositories were segemented in that
> manner.
>
> -Chad
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l [at] lists
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


jeroendedauw at gmail

Jul 21, 2010, 6:10 AM

Post #16 of 23 (2516 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Hey,

The 'semantic extensions' include Validator and Maps, as they are the base
for Semantic Maps, so these should also not get deferred.

Cheers

--
Jeroen De Dauw
* http://blog.bn2vs.com
* http://wiki.bn2vs.com
Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
66 65!
--


On 21 July 2010 14:54, Jeroen De Dauw <jeroendedauw [at] gmail> wrote:

> Hey,
>
> I'm also fine either way. So if no separate queue is set up, I'd
> appropriate it if the semantic* commits where not marked as deferred from
> now on.
>
> Cheers
>
> --
> Jeroen De Dauw
> * http://blog.bn2vs.com
> * http://wiki.bn2vs.com
> Don't panic. Don't be evil. 50 72 6F 67 72 61 6D 6D 69 6E 67 20 34 20 6C 69
> 66 65!
> --
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l [at] lists
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Simetrical+wikilist at gmail

Jul 21, 2010, 8:13 AM

Post #17 of 23 (2516 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

It strikes me that a better solution is to fix whatever tools we're
using to determine what still needs to be reviewed. If someone is
checking all revisions marked as "new" and needs to mark things they
won't review as "deferred" to get them off the list, maybe they should
instead be checking all revisions marked as "new" from particular
paths. Then explicit deferral will not be necessary, and projects
like SMW can go ahead and use Code Review at their own pace without
annoying anyone else.

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


roan.kattouw at gmail

Jul 21, 2010, 9:05 AM

Post #18 of 23 (2514 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

2010/7/21 Aryeh Gregor <Simetrical+wikilist [at] gmail>:
> It strikes me that a better solution is to fix whatever tools we're
> using to determine what still needs to be reviewed.  If someone is
> checking all revisions marked as "new" and needs to mark things they
> won't review as "deferred" to get them off the list, maybe they should
> instead be checking all revisions marked as "new" from particular
> paths.  Then explicit deferral will not be necessary, and projects
> like SMW can go ahead and use Code Review at their own pace without
> annoying anyone else.
>
As far as I know, this is exactly what happens in reality.

As I discussed with a few others at Wikimania, it'd be nice to take
this one step further and allow multiple people to sign off on a
revision, possibly with various types of sign-off, like:
* I read the diff and it looks good
* I tested this and seems to work
* I reviewed the niche part of this rev that I'm an expert on
* I am Tim Starling and I approve this message^Hrevision
* ...

Roan Kattouw (Catrope)

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


Simetrical+wikilist at gmail

Jul 21, 2010, 9:48 AM

Post #19 of 23 (2516 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Wed, Jul 21, 2010 at 12:05 PM, Roan Kattouw <roan.kattouw [at] gmail> wrote:
> As far as I know, this is exactly what happens in reality.

Then why do we need auto-deferral? Just let the things we don't care
about stay new forever.

> As I discussed with a few others at Wikimania, it'd be nice to take
> this one step further and allow multiple people to sign off on a
> revision, possibly with various types of sign-off, like:
> * I read the diff and it looks good
> * I tested this and seems to work
> * I reviewed the niche part of this rev that I'm an expert on
> * I am Tim Starling and I approve this message^Hrevision
> * ...

I think this is a good idea. For simplicity, I'd keep it to one
level, at least at first. The understanding should be that you should
mark it reviewed if you're confident it's correct, and if obvious
errors crop up later, it means people will informally give less weight
to your review. Whether you tested it or just reviewed the diff
should be up to you -- whatever you think it needs.

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


Platonides at gmail

Jul 21, 2010, 2:12 PM

Post #20 of 23 (2517 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

Aryeh Gregor wrote:
>> As I discussed with a few others at Wikimania, it'd be nice to take
>> this one step further and allow multiple people to sign off on a
>> revision, possibly with various types of sign-off, like:
>> * I read the diff and it looks good
>> * I tested this and seems to work
>> * I reviewed the niche part of this rev that I'm an expert on
>> * I am Tim Starling and I approve this message^Hrevision
>> * ...
>
> I think this is a good idea. For simplicity, I'd keep it to one
> level, at least at first. The understanding should be that you should
> mark it reviewed if you're confident it's correct, and if obvious
> errors crop up later, it means people will informally give less weight
> to your review. Whether you tested it or just reviewed the diff
> should be up to you -- whatever you think it needs.

It's not the same. You can have different standards. I see revisions
that are "apparently good", but marking as ok is "This revision is
right" in my book, which in many cases would need actually testing it,
checking spec, and so that I (lazily) don't do. So it keeps as new
instead of as "lightly reviewed".


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


Simetrical+wikilist at gmail

Jul 21, 2010, 2:58 PM

Post #21 of 23 (2501 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Wed, Jul 21, 2010 at 5:12 PM, Platonides <Platonides [at] gmail> wrote:
> It's not the same. You can have different standards. I see revisions
> that are "apparently good", but marking as ok is "This revision is
> right" in my book, which in many cases would need actually testing it,
> checking spec, and so that I (lazily) don't do. So it keeps as new
> instead of as "lightly reviewed".

Is it useful to know that something is lightly reviewed?

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


roan.kattouw at gmail

Jul 21, 2010, 3:51 PM

Post #22 of 23 (2499 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

2010/7/21 Aryeh Gregor <Simetrical+wikilist [at] gmail>:
> On Wed, Jul 21, 2010 at 5:12 PM, Platonides <Platonides [at] gmail> wrote:
>> It's not the same. You can have different standards. I see revisions
>> that are "apparently good", but marking as ok is "This revision is
>> right" in my book, which in many cases would need actually testing it,
>> checking spec, and so that I (lazily) don't do. So it keeps as new
>> instead of as "lightly reviewed".
>
> Is it useful to know that something is lightly reviewed?
>
I would say so, yes. The more people 'lightly review' something, the
less likely it is to have obvious-ish flaws. However, most of the time
the name of the person that reviewed it is the most significant piece
of information, assuming you know the reviewers well.

Roan Kattouw (Catrope)

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


Simetrical+wikilist at gmail

Jul 22, 2010, 8:18 AM

Post #23 of 23 (2487 views)
Permalink
Re: CodeReview auto-deferral regexes [In reply to]

On Wed, Jul 21, 2010 at 6:51 PM, Roan Kattouw <roan.kattouw [at] gmail> wrote:
> I would say so, yes. The more people 'lightly review' something, the
> less likely it is to have obvious-ish flaws. However, most of the time
> the name of the person that reviewed it is the most significant piece
> of information, assuming you know the reviewers well.

Maybe, but I don't think we need actual UI for indicating this. What
we'd really want to know is whether at least one trusted person has
carefully looked at the code and really thinks it's correct. IMO, we
should have a system that encourages people to commit to a change's
correctness, in a way that they can be held accountable (via people
paying less attention to their review) if they miss things too often.
Having a secondary review level that doesn't really mean much because
you only superficially reviewed the code will encourage people to use
that level of review so that they can't really be blamed if the commit
is actually bad, rather than taking responsibility for it.

_______________________________________________
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.