[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Dimitri Glazkov]

On Tue, Oct 13, 2009 at 9:26 AM, Darin Fisher  wrote:
> On Tue, Oct 13, 2009 at 8:21 AM, Adam Barth  wrote:
>>
>> On Tue, Oct 13, 2009 at 8:09 AM, David Levin  wrote:
>> > The webkit api won't help if chromium folks (especially when you change
>> > v8
>> > bindings) don't run the layout tests which is what happened yesterday
>> > and
>> > causes quite a few of our worst problems while gardening.
>>
>> I agree this is essential.
>>
>> > If you're changing the v8 bindings, you're doing it to fix a problem in
>> > chromium, so you must have a build of chomium (and thus be able to run
>> > layout tests).
>>
>> Eric is going to add commit-queue coverage for the V8 bindings, which
>> should help with this problem significantly.
>
> I think this is a great addition, but shouldn't the canary bots have had
> just as
> much of a chance of catching this issue?  Why didn't they catch it?  Is it
> b/c
> of flakiness?  If so, then try bots won't help that much.
> What I think we need to do is beef up the canary bots so that they provide
> equivalent coverage to the mainline Chromium bots.  This will be essential
> if we ever wish to roll WebKit less often (once per week say), which will
> only
> really be possible once we have the WebKit API.
> -Darin

The canaries did catch it. The gardener is also to blame for this.

:DG<

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Darin Fisher]

On Tue, Oct 13, 2009 at 8:21 AM, Adam Barth  wrote:

> On Tue, Oct 13, 2009 at 8:09 AM, David Levin  wrote:
> > The webkit api won't help if chromium folks (especially when you change
> v8
> > bindings) don't run the layout tests which is what happened yesterday and
> > causes quite a few of our worst problems while gardening.
>
> I agree this is essential.
>
> > If you're changing the v8 bindings, you're doing it to fix a problem in
> > chromium, so you must have a build of chomium (and thus be able to run
> > layout tests).
>
> Eric is going to add commit-queue coverage for the V8 bindings, which
> should help with this problem significantly.
>

I think this is a great addition, but shouldn't the canary bots have had
just as
much of a chance of catching this issue?  Why didn't they catch it?  Is it
b/c
of flakiness?  If so, then try bots won't help that much.

What I think we need to do is beef up the canary bots so that they provide
equivalent coverage to the mainline Chromium bots.  This will be essential
if we ever wish to roll WebKit less often (once per week say), which will
only
really be possible once we have the WebKit API.

-Darin



>
> Adam
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Adam Barth]

On Tue, Oct 13, 2009 at 8:09 AM, David Levin  wrote:
> The webkit api won't help if chromium folks (especially when you change v8
> bindings) don't run the layout tests which is what happened yesterday and
> causes quite a few of our worst problems while gardening.

I agree this is essential.

> If you're changing the v8 bindings, you're doing it to fix a problem in
> chromium, so you must have a build of chomium (and thus be able to run
> layout tests).

Eric is going to add commit-queue coverage for the V8 bindings, which
should help with this problem significantly.

Adam

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[David Levin]

The webkit api won't help if chromium folks (especially when you change v8
bindings) don't *run the layout tests *which is what happened yesterday and
causes quite a few of our worst problems while gardening.
If you're changing the v8 bindings, you're doing it to fix a problem in
chromium, so you must have a build of chomium (and thus be able to run
layout tests).

dave

On Mon, Oct 12, 2009 at 11:40 PM, Darin Fisher  wrote:

> On Fri, Sep 25, 2009 at 10:21 AM, Jeremy Orlow wrote:
>
>> On Fri, Sep 25, 2009 at 10:01 AM, Amanda Walker wrote:
>>
>>> On Tue, Sep 22, 2009 at 9:06 PM, Dimitri Glazkov wrote:
>>>
 This all means that we have to be a bit more diligent. We shouldn't be
 paying these unnecessary costs. So, from now on, I propose a fairly
 simple set of new rules:

 1) if you write a Chromium patch for WebKit, you must provide URLs of
 successful trybot runs with your submission. Chromium WebKit reviewers
 will not r+ your patch otherwise. If you can't provide the trybot URLs
 for some reason, please explain in detail why this patch could still
 land.

 2) if the two-sided patch you authored broke the canary and this
 happened with no coordination with the WebKit gardener, you assume
 WebKit gardening responsibility for the next 24 hours.

 Hopefully, these amendments to our existing ways will bring a bit more
 peace and stability to the Chromium land. What do you think?

>>>
>>> I think they're a good start, but they are symptoms of a larger problem.
>>>  "Move fast and clean up messes when they happen" worked great when we had
>>> one big mess a week.  These days, we have multiple ones per day.  And as
>>> good as the try bots and canaries are (kudos to M-A for setting up the try
>>> bots in the first place, and everyone who helps keep the ever-growing herd
>>> of bots running), they don't catch everything.  They don't catch build time
>>> regressions because someone forgot svn:ignores when refactoring where
>>> projects get generated, or accidentally checks something into the wrong
>>> directory (not to pick on anyone in particular, these are just recent
>>> examples).
>>>
>>> I'd add a 3rd principle:
>>>
>>> 3) If you change how chromium is built, however innocuous your change
>>> seems (gyp changes, new libraries, etc.), take extra care and use more
>>> reviewers than usual (preferably including someone intimately acquainted
>>> with the bots, such as maruel, thomasvl, markmentovai, nsylvain, etc.).  If
>>> you're reviewing such a change, don't just look at the diffs, try out the
>>> patch and flag anything that seems out of the ordinary.  "Build breakage"
>>> means more than just "doesn't compile"; it can also mean "overcompiles"
>>> (which kills bot performance) or "requires a clobber unnecessarily."
>>>
>>
>> I had to land a 2 sided patch yesterday.  It blew up an important unit
>> test in a very creative way, and we're still trying to figure out how to
>> clean it up nicely.  In the mean time, we have a dev channel release
>> blocker.  There has to be a better way...
>>
>> Here's a crazy idea:
>>
>> 4) Create a WebKit.next branch.  Have full build bot coverage on it.  All
>> integrations would go through it.  Other than that, only 2 sided patches
>> would land on it.  Whenever everything passes, we'd merge in with the main
>> branch.  We'd try very hard never to let it get more than a day or so out of
>> sync.
>>
>> This would make 2 sided merges (which are often the reason for rushing a
>> DEPS roll--don't want to keep the canary red for too long, otherwise it's
>> very hard to sort things out) much less haphazard.  We could even have it
>> roll the DEPS automatically every time a new webkit.org patch lands, and
>> thus eliminate our need for dedicated canaries.
>>
>> Yeah, it's crazybut I kind of like it.  And yeah, when the WebKit API
>> lands things should be better in terms of others breaking us, but this
>> addresses 2 sided patches...which are not going away any time soon.
>>
>> J
>>
>>
> I think we should just finish the WebKit API :-)
>
> Here's the bug list:
> http://code.google.com/p/chromium/issues/list?q=label:WebKitAPI
>
> I'm looking for volunteers to help take on some of these tasks.
>
> -Darin
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Darin Fisher]

On Fri, Sep 25, 2009 at 10:21 AM, Jeremy Orlow  wrote:

> On Fri, Sep 25, 2009 at 10:01 AM, Amanda Walker wrote:
>
>> On Tue, Sep 22, 2009 at 9:06 PM, Dimitri Glazkov wrote:
>>
>>> This all means that we have to be a bit more diligent. We shouldn't be
>>> paying these unnecessary costs. So, from now on, I propose a fairly
>>> simple set of new rules:
>>>
>>> 1) if you write a Chromium patch for WebKit, you must provide URLs of
>>> successful trybot runs with your submission. Chromium WebKit reviewers
>>> will not r+ your patch otherwise. If you can't provide the trybot URLs
>>> for some reason, please explain in detail why this patch could still
>>> land.
>>>
>>> 2) if the two-sided patch you authored broke the canary and this
>>> happened with no coordination with the WebKit gardener, you assume
>>> WebKit gardening responsibility for the next 24 hours.
>>>
>>> Hopefully, these amendments to our existing ways will bring a bit more
>>> peace and stability to the Chromium land. What do you think?
>>>
>>
>> I think they're a good start, but they are symptoms of a larger problem.
>>  "Move fast and clean up messes when they happen" worked great when we had
>> one big mess a week.  These days, we have multiple ones per day.  And as
>> good as the try bots and canaries are (kudos to M-A for setting up the try
>> bots in the first place, and everyone who helps keep the ever-growing herd
>> of bots running), they don't catch everything.  They don't catch build time
>> regressions because someone forgot svn:ignores when refactoring where
>> projects get generated, or accidentally checks something into the wrong
>> directory (not to pick on anyone in particular, these are just recent
>> examples).
>>
>> I'd add a 3rd principle:
>>
>> 3) If you change how chromium is built, however innocuous your change
>> seems (gyp changes, new libraries, etc.), take extra care and use more
>> reviewers than usual (preferably including someone intimately acquainted
>> with the bots, such as maruel, thomasvl, markmentovai, nsylvain, etc.).  If
>> you're reviewing such a change, don't just look at the diffs, try out the
>> patch and flag anything that seems out of the ordinary.  "Build breakage"
>> means more than just "doesn't compile"; it can also mean "overcompiles"
>> (which kills bot performance) or "requires a clobber unnecessarily."
>>
>
> I had to land a 2 sided patch yesterday.  It blew up an important unit test
> in a very creative way, and we're still trying to figure out how to clean it
> up nicely.  In the mean time, we have a dev channel release blocker.  There
> has to be a better way...
>
> Here's a crazy idea:
>
> 4) Create a WebKit.next branch.  Have full build bot coverage on it.  All
> integrations would go through it.  Other than that, only 2 sided patches
> would land on it.  Whenever everything passes, we'd merge in with the main
> branch.  We'd try very hard never to let it get more than a day or so out of
> sync.
>
> This would make 2 sided merges (which are often the reason for rushing a
> DEPS roll--don't want to keep the canary red for too long, otherwise it's
> very hard to sort things out) much less haphazard.  We could even have it
> roll the DEPS automatically every time a new webkit.org patch lands, and
> thus eliminate our need for dedicated canaries.
>
> Yeah, it's crazybut I kind of like it.  And yeah, when the WebKit API
> lands things should be better in terms of others breaking us, but this
> addresses 2 sided patches...which are not going away any time soon.
>
> J
>
>
I think we should just finish the WebKit API :-)

Here's the bug list:
http://code.google.com/p/chromium/issues/list?q=label:WebKitAPI

I'm looking for volunteers to help take on some of these tasks.

-Darin

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Dimitri Glazkov]

On Tue, Sep 22, 2009 at 6:06 PM, Dimitri Glazkov  wrote:
> Today wasn't a happy day for p...@. He did a seemingly innocuous roll
> that broke the world: selenium, ui tests, layout tests. I am sure it
> was stressful and probably added unnecessary gray to his hair.
>

> 2) writers of patches don't test them properly. In Paul's case, it was
> http://trac.webkit.org/changeset/48639, again by a teammate, and it
> looks like the patch wasn't very thoroughly tested -- it showed a few
> regressions in the canary, but because it had to do with V8 garbage
> collection, the failures were intermittent and seemingly random.
> However, landing it on trunk looked like a shrapnel blast.

Happened again today. Alpha (hclam@) was the most recent victim, along
with the prolonged tree closure and sheriff-firefighting. Here's the
perp: http://trac.webkit.org/changeset/49429. May I suggest a heaping
of WebKit gardening duty as the preventive therapy?

:DG<

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Jeremy Orlow]

On Fri, Sep 25, 2009 at 10:01 AM, Amanda Walker  wrote:

> On Tue, Sep 22, 2009 at 9:06 PM, Dimitri Glazkov wrote:
>
>> This all means that we have to be a bit more diligent. We shouldn't be
>> paying these unnecessary costs. So, from now on, I propose a fairly
>> simple set of new rules:
>>
>> 1) if you write a Chromium patch for WebKit, you must provide URLs of
>> successful trybot runs with your submission. Chromium WebKit reviewers
>> will not r+ your patch otherwise. If you can't provide the trybot URLs
>> for some reason, please explain in detail why this patch could still
>> land.
>>
>> 2) if the two-sided patch you authored broke the canary and this
>> happened with no coordination with the WebKit gardener, you assume
>> WebKit gardening responsibility for the next 24 hours.
>>
>> Hopefully, these amendments to our existing ways will bring a bit more
>> peace and stability to the Chromium land. What do you think?
>>
>
> I think they're a good start, but they are symptoms of a larger problem.
>  "Move fast and clean up messes when they happen" worked great when we had
> one big mess a week.  These days, we have multiple ones per day.  And as
> good as the try bots and canaries are (kudos to M-A for setting up the try
> bots in the first place, and everyone who helps keep the ever-growing herd
> of bots running), they don't catch everything.  They don't catch build time
> regressions because someone forgot svn:ignores when refactoring where
> projects get generated, or accidentally checks something into the wrong
> directory (not to pick on anyone in particular, these are just recent
> examples).
>
> I'd add a 3rd principle:
>
> 3) If you change how chromium is built, however innocuous your change seems
> (gyp changes, new libraries, etc.), take extra care and use more reviewers
> than usual (preferably including someone intimately acquainted with the
> bots, such as maruel, thomasvl, markmentovai, nsylvain, etc.).  If you're
> reviewing such a change, don't just look at the diffs, try out the patch and
> flag anything that seems out of the ordinary.  "Build breakage" means more
> than just "doesn't compile"; it can also mean "overcompiles" (which kills
> bot performance) or "requires a clobber unnecessarily."
>

I had to land a 2 sided patch yesterday.  It blew up an important unit test
in a very creative way, and we're still trying to figure out how to clean it
up nicely.  In the mean time, we have a dev channel release blocker.  There
has to be a better way...

Here's a crazy idea:

4) Create a WebKit.next branch.  Have full build bot coverage on it.  All
integrations would go through it.  Other than that, only 2 sided patches
would land on it.  Whenever everything passes, we'd merge in with the main
branch.  We'd try very hard never to let it get more than a day or so out of
sync.

This would make 2 sided merges (which are often the reason for rushing a
DEPS roll--don't want to keep the canary red for too long, otherwise it's
very hard to sort things out) much less haphazard.  We could even have it
roll the DEPS automatically every time a new webkit.org patch lands, and
thus eliminate our need for dedicated canaries.

Yeah, it's crazybut I kind of like it.  And yeah, when the WebKit API
lands things should be better in terms of others breaking us, but this
addresses 2 sided patches...which are not going away any time soon.

J

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Amanda Walker]

On Tue, Sep 22, 2009 at 9:06 PM, Dimitri Glazkov wrote:

> This all means that we have to be a bit more diligent. We shouldn't be
> paying these unnecessary costs. So, from now on, I propose a fairly
> simple set of new rules:
>
> 1) if you write a Chromium patch for WebKit, you must provide URLs of
> successful trybot runs with your submission. Chromium WebKit reviewers
> will not r+ your patch otherwise. If you can't provide the trybot URLs
> for some reason, please explain in detail why this patch could still
> land.
>
> 2) if the two-sided patch you authored broke the canary and this
> happened with no coordination with the WebKit gardener, you assume
> WebKit gardening responsibility for the next 24 hours.
>
> Hopefully, these amendments to our existing ways will bring a bit more
> peace and stability to the Chromium land. What do you think?
>

I think they're a good start, but they are symptoms of a larger problem.
 "Move fast and clean up messes when they happen" worked great when we had
one big mess a week.  These days, we have multiple ones per day.  And as
good as the try bots and canaries are (kudos to M-A for setting up the try
bots in the first place, and everyone who helps keep the ever-growing herd
of bots running), they don't catch everything.  They don't catch build time
regressions because someone forgot svn:ignores when refactoring where
projects get generated, or accidentally checks something into the wrong
directory (not to pick on anyone in particular, these are just recent
examples).

I'd add a 3rd principle:

3) If you change how chromium is built, however innocuous your change seems
(gyp changes, new libraries, etc.), take extra care and use more reviewers
than usual (preferably including someone intimately acquainted with the
bots, such as maruel, thomasvl, markmentovai, nsylvain, etc.).  If you're
reviewing such a change, don't just look at the diffs, try out the patch and
flag anything that seems out of the ordinary.  "Build breakage" means more
than just "doesn't compile"; it can also mean "overcompiles" (which kills
bot performance) or "requires a clobber unnecessarily."

--Amanda

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Marc-Antoine Ruel]

That wasn't meant to be snarky at all! Contributions are always appreciated.

One thing that is helpful is filing feature request (and assigning
them to me) so I can better prioritize the feature request and not
forget about them.

M-A

On Thu, Sep 24, 2009 at 5:25 PM, Drew Wilson  wrote:
> Why the snark? Is it inappropriate to point out pain points in a proposed
> process without attaching a patch to my email?
> As someone who happily spent an hour+ yesterday tracking down an obscure bug
> in trychange.py, I don't think I'm someone who just sits on the sidelines
> making work for other people.
>
> -atw
>
> On Thu, Sep 24, 2009 at 2:20 PM, Marc-Antoine Ruel 
> wrote:
>>
>> Thanks for proposing yourself in improving gcl.py and trychange.py.
>>
>> On Thu, Sep 24, 2009 at 5:18 PM, Drew Wilson 
>> wrote:
>> > Before we start enforcing penalties for people who screw this up, we
>> > need to
>> > do some tools work to make this less error prone/manual-hackery, I
>> > think.
>> > -atw
>> >
>> > On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart
>> > 
>> > wrote:
>> >>
>> >> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman 
>> >> wrote:
>> >>>
>> >>> Yes.
>> >>> With git:
>> >>> git try -b  --webkit  
>> >>> With gcl:
>> >>> Manually create a patch by concatenating two (chromium + webkit)
>> >>> patches.
>> >>> Make sure that the webkit patches have the right prefixes by using
>> >>> --src-prefix=src/third_party/WebKit
>> >>> --dst-prefix=src/third_party/WebKit on
>> >>> your diff command.
>> >>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow 
>> >>> wrote:
>> 
>>  On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth 
>>  wrote:
>> >
>> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek
>> > 
>> > wrote:
>> > > Is this even possible?  i.e. I had uploaded a WebKit patch on
>> > > codereview but
>> > > none of the patchsets got run on the try server
>> > > http://codereview.chromium.org/178030/show
>> >
>> > It is possible:
>> >
>> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
>> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> > layout_win,layout_mac,layout_linux --root src/third_party
>> 
>>  What about 2 sided patches.  Is it possible to test those?
>> >>
>> >> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.
>> >>  You
>> >> can also synthesize a change with webkit files by creating a file in
>> >> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
>> >> --
>> >> Steve
>> >>
>> >> >> >>
>> >
>> >
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Yaar Schnitman]

I'll fix gcl try to also do 2-sided patches in a single command like git-try
does. Should be an easy port.

On Thu, Sep 24, 2009 at 2:25 PM, Drew Wilson  wrote:

> Why the snark? Is it inappropriate to point out pain points in a proposed
> process without attaching a patch to my email?
> As someone who happily spent an hour+ yesterday tracking down an obscure
> bug in trychange.py, I don't think I'm someone who just sits on the
> sidelines making work for other people.
>
> -atw
>
> On Thu, Sep 24, 2009 at 2:20 PM, Marc-Antoine Ruel wrote:
>
>> Thanks for proposing yourself in improving gcl.py and trychange.py.
>>
>> On Thu, Sep 24, 2009 at 5:18 PM, Drew Wilson 
>> wrote:
>> > Before we start enforcing penalties for people who screw this up, we
>> need to
>> > do some tools work to make this less error prone/manual-hackery, I
>> think.
>> > -atw
>> >
>> > On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart <
>> vand...@chromium.org>
>> > wrote:
>> >>
>> >> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman 
>> wrote:
>> >>>
>> >>> Yes.
>> >>> With git:
>> >>> git try -b  --webkit  
>> >>> With gcl:
>> >>> Manually create a patch by concatenating two (chromium + webkit)
>> patches.
>> >>> Make sure that the webkit patches have the right prefixes by using
>> >>> --src-prefix=src/third_party/WebKit
>> --dst-prefix=src/third_party/WebKit on
>> >>> your diff command.
>> >>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow 
>> >>> wrote:
>> 
>>  On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth 
>> wrote:
>> >
>> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek <
>> j...@chromium.org>
>> > wrote:
>> > > Is this even possible?  i.e. I had uploaded a WebKit patch on
>> > > codereview but
>> > > none of the patchsets got run on the try server
>> > > http://codereview.chromium.org/178030/show
>> >
>> > It is possible:
>> >
>> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try
>> scriptcontext
>> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> > layout_win,layout_mac,layout_linux --root src/third_party
>> 
>>  What about 2 sided patches.  Is it possible to test those?
>> >>
>> >> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.
>>  You
>> >> can also synthesize a change with webkit files by creating a file in
>> >> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
>> >> --
>> >> Steve
>> >>
>> >> >> >>
>> >
>> >
>>
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Drew Wilson]

Why the snark? Is it inappropriate to point out pain points in a proposed
process without attaching a patch to my email?
As someone who happily spent an hour+ yesterday tracking down an obscure bug
in trychange.py, I don't think I'm someone who just sits on the sidelines
making work for other people.

-atw

On Thu, Sep 24, 2009 at 2:20 PM, Marc-Antoine Ruel wrote:

> Thanks for proposing yourself in improving gcl.py and trychange.py.
>
> On Thu, Sep 24, 2009 at 5:18 PM, Drew Wilson 
> wrote:
> > Before we start enforcing penalties for people who screw this up, we need
> to
> > do some tools work to make this less error prone/manual-hackery, I think.
> > -atw
> >
> > On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart  >
> > wrote:
> >>
> >> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman 
> wrote:
> >>>
> >>> Yes.
> >>> With git:
> >>> git try -b  --webkit  
> >>> With gcl:
> >>> Manually create a patch by concatenating two (chromium + webkit)
> patches.
> >>> Make sure that the webkit patches have the right prefixes by using
> >>> --src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit
> on
> >>> your diff command.
> >>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow 
> >>> wrote:
> 
>  On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth 
> wrote:
> >
> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek  >
> > wrote:
> > > Is this even possible?  i.e. I had uploaded a WebKit patch on
> > > codereview but
> > > none of the patchsets got run on the try server
> > > http://codereview.chromium.org/178030/show
> >
> > It is possible:
> >
> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> > layout_win,layout_mac,layout_linux --root src/third_party
> 
>  What about 2 sided patches.  Is it possible to test those?
> >>
> >> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.  You
> >> can also synthesize a change with webkit files by creating a file in
> >> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
> >> --
> >> Steve
> >>
> >> > >>
> >
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Jeremy Orlow]

Maybe this is something that one of the task forces should address?

On Thu, Sep 24, 2009 at 2:20 PM, Marc-Antoine Ruel wrote:

> Thanks for proposing yourself in improving gcl.py and trychange.py.
>
> On Thu, Sep 24, 2009 at 5:18 PM, Drew Wilson 
> wrote:
> > Before we start enforcing penalties for people who screw this up, we need
> to
> > do some tools work to make this less error prone/manual-hackery, I think.
> > -atw
> >
> > On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart  >
> > wrote:
> >>
> >> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman 
> wrote:
> >>>
> >>> Yes.
> >>> With git:
> >>> git try -b  --webkit  
> >>> With gcl:
> >>> Manually create a patch by concatenating two (chromium + webkit)
> patches.
> >>> Make sure that the webkit patches have the right prefixes by using
> >>> --src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit
> on
> >>> your diff command.
> >>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow 
> >>> wrote:
> 
>  On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth 
> wrote:
> >
> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek  >
> > wrote:
> > > Is this even possible?  i.e. I had uploaded a WebKit patch on
> > > codereview but
> > > none of the patchsets got run on the try server
> > > http://codereview.chromium.org/178030/show
> >
> > It is possible:
> >
> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> > layout_win,layout_mac,layout_linux --root src/third_party
> 
>  What about 2 sided patches.  Is it possible to test those?
> >>
> >> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.  You
> >> can also synthesize a change with webkit files by creating a file in
> >> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
> >> --
> >> Steve
> >>
> >> > >>
> >
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Marc-Antoine Ruel]

Thanks for proposing yourself in improving gcl.py and trychange.py.

On Thu, Sep 24, 2009 at 5:18 PM, Drew Wilson  wrote:
> Before we start enforcing penalties for people who screw this up, we need to
> do some tools work to make this less error prone/manual-hackery, I think.
> -atw
>
> On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart 
> wrote:
>>
>> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman  wrote:
>>>
>>> Yes.
>>> With git:
>>> git try -b  --webkit  
>>> With gcl:
>>> Manually create a patch by concatenating two (chromium + webkit) patches.
>>> Make sure that the webkit patches have the right prefixes by using
>>> --src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit on
>>> your diff command.
>>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow 
>>> wrote:

 On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:
>
> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
> wrote:
> > Is this even possible?  i.e. I had uploaded a WebKit patch on
> > codereview but
> > none of the patchsets got run on the try server
> > http://codereview.chromium.org/178030/show
>
> It is possible:
>
> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> layout_win,layout_mac,layout_linux --root src/third_party

 What about 2 sided patches.  Is it possible to test those?
>>
>> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.  You
>> can also synthesize a change with webkit files by creating a file in
>> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
>> --
>> Steve
>>
>> >>
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Drew Wilson]

Before we start enforcing penalties for people who screw this up, we need to
do some tools work to make this less error prone/manual-hackery, I think.
-atw

On Thu, Sep 24, 2009 at 1:56 PM, Steve Vandebogart wrote:

>
> On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman  wrote:
>
>> Yes.
>>
>> With git:git try -b  --webkit  
>>
>> With gcl:
>> Manually create a patch by concatenating two (chromium + webkit) patches.
>> Make sure that the webkit patches have the right prefixes by using
>> --src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit on
>> your diff command.
>>
>> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow wrote:
>>
>>> On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:
>>>

 On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
 wrote:
 > Is this even possible?  i.e. I had uploaded a WebKit patch on
 codereview but
 > none of the patchsets got run on the try server
 > http://codereview.chromium.org/178030/show

 It is possible:

 aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
 --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
 layout_win,layout_mac,layout_linux --root src/third_party

>>>
>>> What about 2 sided patches.  Is it possible to test those?
>>>
>>
> For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.  You
> can also synthesize a change with webkit files by creating a file in
> .svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
> --
> Steve
>
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Steve Vandebogart]

On Thu, Sep 24, 2009 at 1:49 PM, Yaar Schnitman  wrote:

> Yes.
>
> With git:git try -b  --webkit  
>
> With gcl:
> Manually create a patch by concatenating two (chromium + webkit) patches.
> Make sure that the webkit patches have the right prefixes by using
> --src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit on
> your diff command.
>
> On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow  wrote:
>
>> On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:
>>
>>>
>>> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
>>> wrote:
>>> > Is this even possible?  i.e. I had uploaded a WebKit patch on
>>> codereview but
>>> > none of the patchsets got run on the try server
>>> > http://codereview.chromium.org/178030/show
>>>
>>> It is possible:
>>>
>>> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
>>> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>>> layout_win,layout_mac,layout_linux --root src/third_party
>>>
>>
>> What about 2 sided patches.  Is it possible to test those?
>>
>
For gcl, you can also gcl try change -f wekitfile1,webkitfile2,etc.  You can
also synthesize a change with webkit files by creating a file in
.svn/gcl_info/changes and then gcl try chrome_change,webkit_change.
--
Steve

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Yaar Schnitman]

Yes.

With git:git try -b  --webkit  

With gcl:
Manually create a patch by concatenating two (chromium + webkit) patches.
Make sure that the webkit patches have the right prefixes by using
--src-prefix=src/third_party/WebKit --dst-prefix=src/third_party/WebKit on
your diff command.

On Thu, Sep 24, 2009 at 1:43 PM, Jeremy Orlow  wrote:

> On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:
>
>>
>> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
>> wrote:
>> > Is this even possible?  i.e. I had uploaded a WebKit patch on codereview
>> but
>> > none of the patchsets got run on the try server
>> > http://codereview.chromium.org/178030/show
>>
>> It is possible:
>>
>> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
>> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> layout_win,layout_mac,layout_linux --root src/third_party
>>
>
> What about 2 sided patches.  Is it possible to test those?
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Jeremy Orlow]

On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:

>
> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
> wrote:
> > Is this even possible?  i.e. I had uploaded a WebKit patch on codereview
> but
> > none of the patchsets got run on the try server
> > http://codereview.chromium.org/178030/show
>
> It is possible:
>
> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> layout_win,layout_mac,layout_linux --root src/third_party
>

What about 2 sided patches.  Is it possible to test those?

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Adam Barth]

FWIW, that command worked for me, but I have a hybrid checkout:

http://dev.chromium.org/developers/contributing-to-webkit

Adam


On Wed, Sep 23, 2009 at 1:10 PM, John Abd-El-Malek  wrote:
> so I just tried a few experiments.  For some reason, none of the changes in
> third_party\WebKit do an automatic try when uploading.
> When I try to do gcl try with --root src\third_party, the change doesn't
> apply on the server since it tries to patch a diff with
> src\third_party\WebKit... in the filenames in the src\third_party directory.
> I tried without --root, and it doesn't work because then the filenames are
> under WebCore directory
> (see http://codereview.chromium.org/download/issue223008_1.diff) don't have
> src\third_party in them.
> Something seems to be fishy in how the filenames are generates when in the
> WebKit directory.  Marc-Antoine knows this best, so I'm handing it off.
>
> On Wed, Sep 23, 2009 at 10:13 AM, Dimitri Glazkov 
> wrote:
>>
>> BTW, I trying to be snarky in my request. More like begging :)
>>
>> :DG<
>>
>> On Wed, Sep 23, 2009 at 10:08 AM, John Abd-El-Malek 
>> wrote:
>> > Marc-Antonie knows the try scripts best, so to avoid volunteering him
>> > directly, I'll say that I'm sure he can answer any questions from
>> > whoever
>> > does this.  I don't think it'll be much work.
>> >
>> > On Wed, Sep 23, 2009 at 10:04 AM, Dimitri Glazkov 
>> > wrote:
>> >>
>> >> I think this is a great idea! Do we have a Python/gcl/rietveld expert
>> >> who can tackle this?
>> >>
>> >> :DG<
>> >>
>> >> On Wed, Sep 23, 2009 at 9:49 AM, John Abd-El-Malek 
>> >> wrote:
>> >> > I didn't know this was possible.  It seems it will get a lot more
>> >> > usage
>> >> > if
>> >> > it "just works", i.e. the try script grabs these settings
>> >> > automatically
>> >> > from
>> >> > a codereview.settings file.  If we start by putting this file in
>> >> > third_party\WebKit, then people who start with their patch there
>> >> > (also
>> >> > to
>> >> > upload) can use this transparently.  What do you think?
>> >> > On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel
>> >> > 
>> >> > wrote:
>> >> >>
>> >> >> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth 
>> >> >> wrote:
>> >> >> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek
>> >> >> > 
>> >> >> > wrote:
>> >> >> >> Is this even possible?  i.e. I had uploaded a WebKit patch on
>> >> >> >> codereview but
>> >> >> >> none of the patchsets got run on the try server
>> >> >> >> http://codereview.chromium.org/178030/show
>> >> >> >
>> >> >> > It is possible:
>> >> >> >
>> >> >> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try
>> >> >> > scriptcontext
>> >> >> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> >> >> > layout_win,layout_mac,layout_linux --root src/third_party
>> >> >>
>> >> >> I know the format is not very user friendly but it is definitely
>> >> >> possible. You probably can skip --use_svn and --svn_repo or at least
>> >> >> use the --http stuff instead, see go/chrometryserver.
>> >> >>
>> >> >> You could want to use both the layout tests and the normal tests so
>> >> >> you can send the patch to all the slaves at the same time, e.g.
>> >> >> --bot win,linux,mac,layout_win,layout_mac,layout_linux
>> >> >>
>> >> >> If the layout tests try slaves get overused, I'll add more slaves.
>> >> >>
>> >> >> M-A
>> >> >
>> >> >
>> >
>> >
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[John Abd-El-Malek]

so I just tried a few experiments.  For some reason, none of the changes in
third_party\WebKit do an automatic try when uploading.
When I try to do gcl try with --root src\third_party, the change doesn't
apply on the server since it tries to patch a diff with
src\third_party\WebKit... in the filenames in the src\third_party directory.

I tried without --root, and it doesn't work because then the filenames are
under WebCore directory (see
http://codereview.chromium.org/download/issue223008_1.diff) don't have
src\third_party in them.

Something seems to be fishy in how the filenames are generates when in the
WebKit directory.  Marc-Antoine knows this best, so I'm handing it off.

On Wed, Sep 23, 2009 at 10:13 AM, Dimitri Glazkov wrote:

> BTW, I trying to be snarky in my request. More like begging :)
>
> :DG<
>
> On Wed, Sep 23, 2009 at 10:08 AM, John Abd-El-Malek 
> wrote:
> > Marc-Antonie knows the try scripts best, so to avoid volunteering him
> > directly, I'll say that I'm sure he can answer any questions from whoever
> > does this.  I don't think it'll be much work.
> >
> > On Wed, Sep 23, 2009 at 10:04 AM, Dimitri Glazkov 
> > wrote:
> >>
> >> I think this is a great idea! Do we have a Python/gcl/rietveld expert
> >> who can tackle this?
> >>
> >> :DG<
> >>
> >> On Wed, Sep 23, 2009 at 9:49 AM, John Abd-El-Malek 
> >> wrote:
> >> > I didn't know this was possible.  It seems it will get a lot more
> usage
> >> > if
> >> > it "just works", i.e. the try script grabs these settings
> automatically
> >> > from
> >> > a codereview.settings file.  If we start by putting this file in
> >> > third_party\WebKit, then people who start with their patch there (also
> >> > to
> >> > upload) can use this transparently.  What do you think?
> >> > On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel <
> mar...@chromium.org>
> >> > wrote:
> >> >>
> >> >> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth 
> >> >> wrote:
> >> >> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek <
> j...@chromium.org>
> >> >> > wrote:
> >> >> >> Is this even possible?  i.e. I had uploaded a WebKit patch on
> >> >> >> codereview but
> >> >> >> none of the patchsets got run on the try server
> >> >> >> http://codereview.chromium.org/178030/show
> >> >> >
> >> >> > It is possible:
> >> >> >
> >> >> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try
> scriptcontext
> >> >> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> >> >> > layout_win,layout_mac,layout_linux --root src/third_party
> >> >>
> >> >> I know the format is not very user friendly but it is definitely
> >> >> possible. You probably can skip --use_svn and --svn_repo or at least
> >> >> use the --http stuff instead, see go/chrometryserver.
> >> >>
> >> >> You could want to use both the layout tests and the normal tests so
> >> >> you can send the patch to all the slaves at the same time, e.g.
> >> >> --bot win,linux,mac,layout_win,layout_mac,layout_linux
> >> >>
> >> >> If the layout tests try slaves get overused, I'll add more slaves.
> >> >>
> >> >> M-A
> >> >
> >> >
> >
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Dimitri Glazkov]

BTW, I trying to be snarky in my request. More like begging :)

:DG<

On Wed, Sep 23, 2009 at 10:08 AM, John Abd-El-Malek  wrote:
> Marc-Antonie knows the try scripts best, so to avoid volunteering him
> directly, I'll say that I'm sure he can answer any questions from whoever
> does this.  I don't think it'll be much work.
>
> On Wed, Sep 23, 2009 at 10:04 AM, Dimitri Glazkov 
> wrote:
>>
>> I think this is a great idea! Do we have a Python/gcl/rietveld expert
>> who can tackle this?
>>
>> :DG<
>>
>> On Wed, Sep 23, 2009 at 9:49 AM, John Abd-El-Malek 
>> wrote:
>> > I didn't know this was possible.  It seems it will get a lot more usage
>> > if
>> > it "just works", i.e. the try script grabs these settings automatically
>> > from
>> > a codereview.settings file.  If we start by putting this file in
>> > third_party\WebKit, then people who start with their patch there (also
>> > to
>> > upload) can use this transparently.  What do you think?
>> > On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel 
>> > wrote:
>> >>
>> >> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth 
>> >> wrote:
>> >> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
>> >> > wrote:
>> >> >> Is this even possible?  i.e. I had uploaded a WebKit patch on
>> >> >> codereview but
>> >> >> none of the patchsets got run on the try server
>> >> >> http://codereview.chromium.org/178030/show
>> >> >
>> >> > It is possible:
>> >> >
>> >> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
>> >> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> >> > layout_win,layout_mac,layout_linux --root src/third_party
>> >>
>> >> I know the format is not very user friendly but it is definitely
>> >> possible. You probably can skip --use_svn and --svn_repo or at least
>> >> use the --http stuff instead, see go/chrometryserver.
>> >>
>> >> You could want to use both the layout tests and the normal tests so
>> >> you can send the patch to all the slaves at the same time, e.g.
>> >> --bot win,linux,mac,layout_win,layout_mac,layout_linux
>> >>
>> >> If the layout tests try slaves get overused, I'll add more slaves.
>> >>
>> >> M-A
>> >
>> >
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[John Abd-El-Malek]

Marc-Antonie knows the try scripts best, so to avoid volunteering him
directly, I'll say that I'm sure he can answer any questions from whoever
does this.  I don't think it'll be much work.

On Wed, Sep 23, 2009 at 10:04 AM, Dimitri Glazkov wrote:

> I think this is a great idea! Do we have a Python/gcl/rietveld expert
> who can tackle this?
>
> :DG<
>
> On Wed, Sep 23, 2009 at 9:49 AM, John Abd-El-Malek 
> wrote:
> > I didn't know this was possible.  It seems it will get a lot more usage
> if
> > it "just works", i.e. the try script grabs these settings automatically
> from
> > a codereview.settings file.  If we start by putting this file in
> > third_party\WebKit, then people who start with their patch there (also to
> > upload) can use this transparently.  What do you think?
> > On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel 
> > wrote:
> >>
> >> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth 
> wrote:
> >> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
> >> > wrote:
> >> >> Is this even possible?  i.e. I had uploaded a WebKit patch on
> >> >> codereview but
> >> >> none of the patchsets got run on the try server
> >> >> http://codereview.chromium.org/178030/show
> >> >
> >> > It is possible:
> >> >
> >> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> >> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> >> > layout_win,layout_mac,layout_linux --root src/third_party
> >>
> >> I know the format is not very user friendly but it is definitely
> >> possible. You probably can skip --use_svn and --svn_repo or at least
> >> use the --http stuff instead, see go/chrometryserver.
> >>
> >> You could want to use both the layout tests and the normal tests so
> >> you can send the patch to all the slaves at the same time, e.g.
> >> --bot win,linux,mac,layout_win,layout_mac,layout_linux
> >>
> >> If the layout tests try slaves get overused, I'll add more slaves.
> >>
> >> M-A
> >
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Dimitri Glazkov]

I think this is a great idea! Do we have a Python/gcl/rietveld expert
who can tackle this?

:DG<

On Wed, Sep 23, 2009 at 9:49 AM, John Abd-El-Malek  wrote:
> I didn't know this was possible.  It seems it will get a lot more usage if
> it "just works", i.e. the try script grabs these settings automatically from
> a codereview.settings file.  If we start by putting this file in
> third_party\WebKit, then people who start with their patch there (also to
> upload) can use this transparently.  What do you think?
> On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel 
> wrote:
>>
>> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth  wrote:
>> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
>> > wrote:
>> >> Is this even possible?  i.e. I had uploaded a WebKit patch on
>> >> codereview but
>> >> none of the patchsets got run on the try server
>> >> http://codereview.chromium.org/178030/show
>> >
>> > It is possible:
>> >
>> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
>> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
>> > layout_win,layout_mac,layout_linux --root src/third_party
>>
>> I know the format is not very user friendly but it is definitely
>> possible. You probably can skip --use_svn and --svn_repo or at least
>> use the --http stuff instead, see go/chrometryserver.
>>
>> You could want to use both the layout tests and the normal tests so
>> you can send the patch to all the slaves at the same time, e.g.
>> --bot win,linux,mac,layout_win,layout_mac,layout_linux
>>
>> If the layout tests try slaves get overused, I'll add more slaves.
>>
>> M-A
>
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[John Abd-El-Malek]

I didn't know this was possible.  It seems it will get a lot more usage if
it "just works", i.e. the try script grabs these settings automatically from
a codereview.settings file.  If we start by putting this file in
third_party\WebKit, then people who start with their patch there (also to
upload) can use this transparently.  What do you think?
On Wed, Sep 23, 2009 at 7:42 AM, Marc-Antoine Ruel wrote:

> On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth  wrote:
> > On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
> wrote:
> >> Is this even possible?  i.e. I had uploaded a WebKit patch on codereview
> but
> >> none of the patchsets got run on the try server
> >> http://codereview.chromium.org/178030/show
> >
> > It is possible:
> >
> > aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> > --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> > layout_win,layout_mac,layout_linux --root src/third_party
>
> I know the format is not very user friendly but it is definitely
> possible. You probably can skip --use_svn and --svn_repo or at least
> use the --http stuff instead, see go/chrometryserver.
>
> You could want to use both the layout tests and the normal tests so
> you can send the patch to all the slaves at the same time, e.g.
> --bot win,linux,mac,layout_win,layout_mac,layout_linux
>
> If the layout tests try slaves get overused, I'll add more slaves.
>
> M-A
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Nicolas Sylvain]

On Wed, Sep 23, 2009 at 8:55 AM, Paweł Hajdan Jr.
wrote:

> On Tue, Sep 22, 2009 at 18:06, Dimitri Glazkov wrote:
>
>> Today wasn't a happy day for p...@. He did a seemingly innocuous roll
>> that broke the world: selenium, ui tests, layout tests. I am sure it
>> was stressful and probably added unnecessary gray to his hair.
>
>
> How about running ui and selenium tests on the canary bot?
>
We run selenium already. And UI is covered by try server,
but I'll make the change to also run the ui tests on the canary.

Nicolas


> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Paweł Hajdan Jr .]

On Tue, Sep 22, 2009 at 18:06, Dimitri Glazkov  wrote:

> Today wasn't a happy day for p...@. He did a seemingly innocuous roll
> that broke the world: selenium, ui tests, layout tests. I am sure it
> was stressful and probably added unnecessary gray to his hair.


How about running ui and selenium tests on the canary bot?

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Marc-Antoine Ruel]

On Tue, Sep 22, 2009 at 9:35 PM, Adam Barth  wrote:
> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek  wrote:
>> Is this even possible?  i.e. I had uploaded a WebKit patch on codereview but
>> none of the patchsets got run on the try server
>> http://codereview.chromium.org/178030/show
>
> It is possible:
>
> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> layout_win,layout_mac,layout_linux --root src/third_party

I know the format is not very user friendly but it is definitely
possible. You probably can skip --use_svn and --svn_repo or at least
use the --http stuff instead, see go/chrometryserver.

You could want to use both the layout tests and the normal tests so
you can send the patch to all the slaves at the same time, e.g.
--bot win,linux,mac,layout_win,layout_mac,layout_linux

If the layout tests try slaves get overused, I'll add more slaves.

M-A

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Dimitri Glazkov]

> 1) writers of patches don't mention that the patch is two-sided and
> will break Chromium if landed prematurely. I don't have to go far for
> an example. Commit queue bot landed
> http://trac.webkit.org/changeset/48659 a few minutes ago and broke the
> canary. This means that the canary will be red all night and any
> subsequent regressions will either not be noticed or create more
> complications.

As I found out later that night (as I was fixing the bustage), this
particular break wasn't due to the two-sided nature of the patch. It
was a couple of missing includes. Apologies for mis-placing it into
this category. This one was from the #2 pile :)

:DG<

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Nicolas Sylvain]

On Tue, Sep 22, 2009 at 6:35 PM, Adam Barth  wrote:

>
> On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek 
> wrote:
> > Is this even possible?  i.e. I had uploaded a WebKit patch on codereview
> but
> > none of the patchsets got run on the try server
> > http://codereview.chromium.org/178030/show
>
> It is possible:
>
> aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
> --use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
> layout_win,layout_mac,layout_linux --root src/third_party
>

And if you use git, there was another thread today about this on
chromium-dev :
http://groups.google.com/group/chromium-dev/browse_thread/thread/c481ecc1842ddfe4#


Nicolas


>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Adam Barth]

On Tue, Sep 22, 2009 at 6:25 PM, John Abd-El-Malek  wrote:
> Is this even possible?  i.e. I had uploaded a WebKit patch on codereview but
> none of the patchsets got run on the try server
> http://codereview.chromium.org/178030/show

It is possible:

aba...@zenque:~/svn/kr/src/third_party/WebKit$ gcl try scriptcontext
--use_svn --svn_repo=svn://svn.chromium.org/chrome-try/try --bot
layout_win,layout_mac,layout_linux --root src/third_party

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[John Abd-El-Malek]

On Tue, Sep 22, 2009 at 6:06 PM, Dimitri Glazkov wrote:

>
> Today wasn't a happy day for p...@. He did a seemingly innocuous roll
> that broke the world: selenium, ui tests, layout tests. I am sure it
> was stressful and probably added unnecessary gray to his hair.
>
> Stuff like this happens to WebKit gardeners. We're used to breakages
> upstream. That's the cost of being unforked, right?
>
> The problem however, is that since we unforked, most of these
> breakages and regressions are caused by fellow teammates. There are
> two major issues:
>
> 1) writers of patches don't mention that the patch is two-sided and
> will break Chromium if landed prematurely. I don't have to go far for
> an example. Commit queue bot landed
> http://trac.webkit.org/changeset/48659 a few minutes ago and broke the
> canary. This means that the canary will be red all night and any
> subsequent regressions will either not be noticed or create more
> complications.
>
> 2) writers of patches don't test them properly. In Paul's case, it was
> http://trac.webkit.org/changeset/48639, again by a teammate, and it
> looks like the patch wasn't very thoroughly tested -- it showed a few
> regressions in the canary, but because it had to do with V8 garbage
> collection, the failures were intermittent and seemingly random.
> However, landing it on trunk looked like a shrapnel blast.
>
> This all means that we have to be a bit more diligent. We shouldn't be
> paying these unnecessary costs. So, from now on, I propose a fairly
> simple set of new rules:
>
> 1) if you write a Chromium patch for WebKit, you must provide URLs of
> successful trybot runs with your submission. Chromium WebKit reviewers
> will not r+ your patch otherwise. If you can't provide the trybot URLs
> for some reason, please explain in detail why this patch could still
> land.
>

Is this even possible?  i.e. I had uploaded a WebKit patch on codereview but
none of the patchsets got run on the try server
http://codereview.chromium.org/178030/show

>
> 2) if the two-sided patch you authored broke the canary and this
> happened with no coordination with the WebKit gardener, you assume
> WebKit gardening responsibility for the next 24 hours.
>
> Hopefully, these amendments to our existing ways will bring a bit more
> peace and stability to the Chromium land. What do you think?
>
> :DG<
>
> :DG<
>
> >
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Jeremy Orlow]

On Tue, Sep 22, 2009 at 6:06 PM, Dimitri Glazkov wrote:

>
> Today wasn't a happy day for p...@. He did a seemingly innocuous roll
> that broke the world: selenium, ui tests, layout tests. I am sure it
> was stressful and probably added unnecessary gray to his hair.
>
> Stuff like this happens to WebKit gardeners. We're used to breakages
> upstream. That's the cost of being unforked, right?
>
> The problem however, is that since we unforked, most of these
> breakages and regressions are caused by fellow teammates. There are
> two major issues:
>
> 1) writers of patches don't mention that the patch is two-sided and
> will break Chromium if landed prematurely. I don't have to go far for
> an example. Commit queue bot landed
> http://trac.webkit.org/changeset/48659 a few minutes ago and broke the
> canary. This means that the canary will be red all night and any
> subsequent regressions will either not be noticed or create more
> complications.
>
> 2) writers of patches don't test them properly. In Paul's case, it was
> http://trac.webkit.org/changeset/48639, again by a teammate, and it
> looks like the patch wasn't very thoroughly tested -- it showed a few
> regressions in the canary, but because it had to do with V8 garbage
> collection, the failures were intermittent and seemingly random.
> However, landing it on trunk looked like a shrapnel blast.
>
> This all means that we have to be a bit more diligent. We shouldn't be
> paying these unnecessary costs. So, from now on, I propose a fairly
> simple set of new rules:
>
> 1) if you write a Chromium patch for WebKit, you must provide URLs of
> successful trybot runs with your submission. Chromium WebKit reviewers
> will not r+ your patch otherwise. If you can't provide the trybot URLs
> for some reason, please explain in detail why this patch could still
> land.
>

This will take some getting used to, but is probably for the best.  Where
are the instructions for trying WebKit and 2 sided patches again?


> 2) if the two-sided patch you authored broke the canary and this
> happened with no coordination with the WebKit gardener, you assume
> WebKit gardening responsibility for the next 24 hours.
>

Definitely.  I also think the gardener reserves the right to revert any
WebKit patch landed by a Chromium contributor.


> Hopefully, these amendments to our existing ways will bring a bit more
> peace and stability to the Chromium land. What do you think?
>

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---

[chromium-dev] Re: WebKit Hygiene: Please be kind. Test your patches. Warn about two-sided patches.

[Adam Barth]

On Tue, Sep 22, 2009 at 6:06 PM, Dimitri Glazkov  wrote:
> 1) if you write a Chromium patch for WebKit, you must provide URLs of
> successful trybot runs with your submission. Chromium WebKit reviewers
> will not r+ your patch otherwise. If you can't provide the trybot URLs
> for some reason, please explain in detail why this patch could still
> land.

We can make this easier by doing this automatically when you post a
patch with bugzilla-tool.

> 2) if the two-sided patch you authored broke the canary and this
> happened with no coordination with the WebKit gardener, you assume
> WebKit gardening responsibility for the next 24 hours.

This seems slightly drastic, but I agree we need a better system for
knowing not to land two-sided patches.

Adam

--~--~-~--~~~---~--~~
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev
-~--~~~~--~~--~--~---