On Sun, Apr 25, 2010 at 4:36 AM, Ryan Hill <firstname.lastname@example.org> wrote:
> On Sun, 25 Apr 2010 13:11:11 +0300
> Petteri Räty <email@example.com> 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.
> 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