Re: [gentoo-dev] Re: Requiring two sets of eyes for all eclass commits

[Alec Warner]

On Sun, Apr 25, 2010 at 4:36 AM, Ryan Hill <dirtyepic@g.o> wrote:
> On Sun, 25 Apr 2010 13:11:11 +0300
> Petteri Räty <betelgeuse@g.o> wrote:
>
>> On 04/25/2010 01:06 PM, Ryan Hill wrote:
>
>> > I think it's a good idea to strongly encourage it, but actually forcing it
>> > through cvs?  No thanks.  I'm not tracking down another dev just to fix a
>> > spelling mistake. :P
>>
>> How did the spelling mistake get there in the first place? A review
>> system should reduce having them in the first place.
>
> People make mistakes.  Anyways my point is that requiring a peer review for
> trivial changes is just unneeded bureaucracy.  Even for non-trivial changes,
> it doesn't make sense to force someone who knows their eclass inside out and
> knows what they're doing to get a review from someone else who may not have a
> clue.  I'm not saying that peer-review shouldn't be done; it's a very good
> idea, especially if you're new, or unsure of your changes, or you have a team
> consisting of more than one person.  In fact I would support a policy that
> said eclasses need to be reviewed before committing.  But enforcing it through
> cvs is never going to fly.  Just use common sense.

Sure it will; you just need to create the tools with flexibility in
mind.  For instance:

1) Require peer review on all eclasses
2) Do not require peer review for changes less than N lines
3) enable a commiter to over-ride the review with some kind of option
(--force or similar)
4) enable an eclass-owner to opt-out of the review process entirely
with some kind of flag.

I am not aware of how robust the pre-commit hooks in cvs are so I
cannot comment on how easy these things are to implement.

-A

>
> If we were having ongoing issues with people breaking eclasses then I could
> see where you're coming from.  But as it is, it's a non-problem.
>
>
> --
> fonts,                                            by design, by neglect
> gcc-porting,                              for a fact or just for effect
> wxwidgets @ gentoo     EFFD 380E 047A 4B51 D2BD C64F 8AA8 8346 F9A4 0662
>