PSA: don't use drover

[Ojan Vafai]

It seems to have some bugs where it doesn't readd deleted files. You're better off just using git revert, made especially friendly by the lack of ChangeLog files.

https://code.google.com/p/chromium/issues/detail?id=235266 for the details.

[Takayoshi Kochi]

The bug seems to have merged to https://code.google.com/p/chromium/issues/detail?id=64643, which has lived very long.

FYI I saw similar thing yesterday - Blink r148985 had some added files but the revert r148987 didn't delete the added files.
The revert just reverted .gypi which wasn't new and it works anyway, I left undeleted files as is to work on fixing it in place.

But next time I'll use git revert.

On Thu, Apr 25, 2013 at 8:44 AM, Ojan Vafai <oj...@chromium.org> wrote:

It seems to have some bugs where it doesn't readd deleted files. You're better off just using git revert, made especially friendly by the lack of ChangeLog files.

https://code.google.com/p/chromium/issues/detail?id=235266 for the details.

--
Takayoshi Kochi

[Ojan Vafai]

For those that don't know, git revert is super fast and easy to use.

1. git log and find the hash of the commit you want to revert.

2. git revert hash

Make sure to add details to the revert commit description pointing to what broke (e.g. to the stdio of a bot). Then do the regular git cl shenanigans to get your patch landed.

[Robert Kroeger]

Should people continue to use drover to down integrate release/beta blockers per http://www.chromium.org/developers/how-tos/drover?

[Dominic Mazzoni]

On Wed, Apr 24, 2013 at 4:44 PM, Ojan Vafai <oj...@chromium.org> wrote:

It seems to have some bugs where it doesn't readd deleted files. You're better off just using git revert, made especially friendly by the lack of ChangeLog files.

Perhaps better advice would be "always double-check drover's revert before committing it". I still find drover to be a lot more convenient than "git revert", and it works fine 90% of the time.

- Dominic

[Ojan Vafai]

On Thu, Apr 25, 2013 at 7:41 AM, Robert Kroeger <rjkr...@chromium.org> wrote:

Should people continue to use drover to down integrate release/beta blockers per http://www.chromium.org/developers/how-tos/drover?

I think it's less of a problem for integrating patches into release branches because it's rare that those patches add new directories. They're almost always targeted fixes for specific bugs. So, I think it's still fine to use drover for integrating into branches.

Blink commits very often add/remove directories for LayoutTest expectations.

 

On Thu, Apr 25, 2013 at 10:23 AM, Dominic Mazzoni <dmaz...@chromium.org> wrote:

On Wed, Apr 24, 2013 at 4:44 PM, Ojan Vafai <oj...@chromium.org> wrote:

It seems to have some bugs where it doesn't readd deleted files. You're better off just using git revert, made especially friendly by the lack of ChangeLog files.

Perhaps better advice would be "always double-check drover's revert before committing it". I still find drover to be a lot more convenient than "git revert", and it works fine 90% of the time.

If that's what you prefer to do, go for it. I didn't mean this to be a prescriptive thing everyone needs to do. Personally, I find git revert more convenient, and it's definitely faster.

 

[Stephen White]

On Wed, Apr 24, 2013 at 10:38 PM, Ojan Vafai <oj...@chromium.org> wrote:

For those that don't know, git revert is super fast and easy to use.

1. git log and find the hash of the commit you want to revert.

2. git revert hash

BTW, if you have the git-svn metadata in your checkout (which a "fetch blink" checkout does by default, I think), you can use:

git revert `git svn find-rev r######`

to do this in one step.

Stephen

[Nils Barth]

[Snip: PSA: don't use drover]

For the record (and people reading archives):

t

his appears

to be fixed (it's ok to use drover to revert).

The issue was:

Issue 64643: Drover fails to revert a CL that deletes a directory.

...but this should be fixed (by Jochen) as of CL 22814005.

[Eric Seidel]

Last time I used drover I was sad that it didn't update the bug/issue
which it was rolling out (so the author knew it had been rolled out).

I guess now that we're back to using it for Blink again, I should
probably go fix that in drover...

[Jochen Eisinger]

On Fri, Aug 23, 2013 at 8:14 AM, Eric Seidel <ese...@chromium.org> wrote:

Last time I used drover I was sad that it didn't update the bug/issue
which it was rolling out (so the author knew it had been rolled out).

It TBR's the author, so the author should be aware.

 

I guess now that we're back to using it for Blink again, I should
probably go fix that in drover...

Since it's creating a regular CL, adding the correct BUG= field should be enough

best

-jochen