PSA: don't use drover

Showing 1-10 of 10 messages
PSA: don't use drover Ojan Vafai 4/24/13 4:44 PM
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.

Re: [blink-dev] PSA: don't use drover Takayoshi Kochi 4/24/13 7:20 PM
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.




--
Takayoshi Kochi
Re: [blink-dev] PSA: don't use drover Ojan Vafai 4/24/13 7:38 PM
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.
Re: [blink-dev] PSA: don't use drover Robert 4/25/13 7:41 AM

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


On Wednesday, April 24, 2013, Ojan Vafai 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

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.


On Wed, Apr 24, 2013 at 7:20 PM, Takayoshi Kochi <kochi@chromium.org> wrote:
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 <ojan@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.




--
Takayoshi Kochi

Re: [blink-dev] PSA: don't use drover Dominic Mazzoni 4/25/13 10:23 AM
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

Re: [blink-dev] PSA: don't use drover Ojan Vafai 4/25/13 10:56 AM
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.
 

Re: [blink-dev] PSA: don't use drover Stephen 5/11/13 6:56 AM
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


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.


On Wed, Apr 24, 2013 at 7:20 PM, Takayoshi Kochi <ko...@chromium.org> wrote:
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.




--
Takayoshi Kochi


Re: [blink-dev] PSA: don't use drover Nils Barth 8/22/13 11:09 PM
[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.
Re: [blink-dev] PSA: don't use drover Eric Seidel 8/22/13 11:14 PM
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...
Re: [blink-dev] PSA: don't use drover Jochen Eisinger 8/22/13 11:23 PM



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
 

On Thu, Aug 22, 2013 at 11:09 PM, Nils Barth <nba...@chromium.org> wrote:
> [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.