I'd like to add two points.

- If you haven't seen my original email, that's normal, it was on an
internal mailing list. It wasn't on a nice tone I won't repeat it
here.
- The try server is still not accessible externally. So saying to use
it externally is a bit moot. ;) That being said, I'm actively working
on open sourcing it. Once that is done, anyone can start one, even
locally.

M-A

2008/10/10  <[EMAIL PROTECTED]>:
>
> I agree with Nicholas's definition of what is allowed to TBR: things that
> are not code changes.  To expand on his list, I think this also includes
> svn prop changes and string translation dumps (xtb file updates).
>
> Historically, we have checked in code TBR on the merge branch during the
> initial phases of getting it to compile and link.  When we were using
> gvn, this still meant you had to send an email.  Whoever the TBR was sent
> to is still responsible for reviewing the change and making sure it's
> correct.  Any feedback on the change still needs to be addressed in a
> follow up change.  The idea being that getting merge branch to
> compile/link shouldn't block on the review since there are a lot of small
> things to do.  I don't think the intent was ever that the merge would be
> reviewed only when landed.
>
> I think we were a bit sloppy with this in the most recent merge.  This is
> possibly because it is tempting to just svn commit with TBR in the commit
> description.  This means no email gets sent and the patch is never
> actually reviewed.  Part of the difficulty is that Reitveld can't handle
> large changes, so it requires manually emailing the viewvc link after the
> commit.
>
> I think when the next merge happens, we just need to be more careful of
> this and try to make sure there's an email associated with every commit.
> If we want to enforce it, we could make an svn commit hook.  The svn hook
> would make sure there's either a Reitveld URL or a TBR in the commit log.
> If there's a TBR, it would send an email to the viewvc link.
>
> tony
>
>
> On Fri, 10 Oct 2008, Nicolas Sylvain wrote:
>
>> On Fri, Oct 10, 2008 at 11:37 AM, Ojan Vafai <[EMAIL PROTECTED]> wrote:
>>
>> > I hope not to start a flame-war here, but I'd like to see written down
>> > somewhere our team policy on committing code. There are some issues that
>> > seem underspecified to me. These are of course just my feelings on these
>> > issues. I hope that coming out of this discussion we can agree on a formal
>> > policy.
>> >
>> >    1. Use the trybots: It's at the point where I think that no one should
>> >    *ever* commit code without at least looking at the results of the 
>> > trybots,
>> >    unless it is an emergency fix for a closed tree (or of course if the 
>> > trybots
>> >    are down). I imagine there won't be much disagreement here and we can 
>> > just
>> >    add this to the appropriate documentation on the Sites page.
>> >    2. Don't
>> >    TBR: I see inconsistency with the team culture around what is 
>> > acceptable to TBR. My experience with the rest of Google is that the 
>> > *only* acceptable changes to TBR are ones that fix closed trees. I would 
>> > feel a lot more comfortable if we had a hard rule like that, but I 
>> > understand others feel differently. In either case, can we generate a hard 
>> > list of the things that are acceptable toTBR?
>> >    3. Watch the waterfall: Noone should ever commit code unless they can
>> >    stick around for the next hour to make sure they didn't cause 
>> > regressions,
>> >    or unless they can ask someone else to monitor the tree for them and act
>> >    appropriately. Not doing one of those two things means that when your
>> >    checkin inadvertently breaks the build it falls on the shoulders of 
>> > either
>> >    the sheriff or whoever happens to be online if it's after hours.
>> >
>> >
>> I agree with all this. To answer your question about "what is acceptable to
>> TBR". Everything that has code should not be TBR. On the other hand I would
>> personally not mind if someone changes the DEPS file, the test_fixable.txt
>> list or the VERSION file with a TBR checkin.
>>
>> What do we do for branches? My understanding was that code reviews for
>> branches were not mandatory, since the code has to be reviewed anyway when
>> it's merge back to trunk.   Is it true?  How do you proceed for the webkit
>> merge branch?
>>
>> Nicolas
>>
>>
>> >    1.
>> >
>> > This all piggy-backs on Marc-Antione's email yesterday about keeping the
>> > tree green. It is possible to keep the tree considerably more green than we
>> > currently do and I think the above would be an enormous step in that
>> > direction. Keeping the tree green makes our team globally more efficient 
>> > and
>> > keeps our sheriffs from hating their jobs. :)
>> >
>> > Thoughts?
>> >
>> > Ojan
>> >
>> > >
>> >
>>
>> >
>>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Chromium-dev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/chromium-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to