kill --downloads-new-ui

kill --downloads-new-ui


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=143630

[benjhayden, 2012-06-19 17:03:04]

PTAL

[asanka, 2012-06-19 17:06:05]

You'll also need to update chrome/browser/about_flags.cc

[Randy Smith (Not in Mondays), 2012-06-19 17:27:42]

Basic idea LGTM; I'll rely on Asanka for the code review.

[benjhayden, 2012-06-19 17:53:03]

On 2012/06/19 17:06:05, asanka wrote:
> You'll also need to update chrome/browser/about_flags.cc

See PS2

[cbentzel, 2012-06-19 18:03:33]

Happy with concept; will be ignorant on details. You could probably
restrict this to asanka and appropriate OWNERS only.

On Tue, Jun 19, 2012 at 1:53 PM, <benjhayden@chromium.org> wrote:

> On 2012/06/19 17:06:05, asanka wrote:
>
>> You'll also need to update chrome/browser/about_flags.cc
>>
>
> See PS2
>
>
http://codereview.chromium.**org/10573012/<http://codereview.chromium.org/105...
>

[asanka, 2012-06-19 18:06:53]

You'll also want to run extract_actions.py and include the updated
chromeactions.txt file.

LGTM

[benjhayden, 2012-06-19 19:17:19]

On 2012/06/19 18:06:53, asanka wrote:
> You'll also want to run extract_actions.py and include the updated
> chromeactions.txt file.
> 
> LGTM

Um, so I did that and it just added an unrelated action
(BadMessageTerminate_RDH).
So I moved chromeactions.txt out of the way, touched it to make an empty file,
and re-ran extract_actions.py, and it "removed" a ton of stuff including
downloads-new-ui.
Should I commit it with removing all the other stuff, or hand-edit it to just
remove downloads-new-ui?
Or do we really want to remove downloads-new-ui? How is chromeactions.txt used?

[asanka, 2012-06-19 20:06:43]

On 2012/06/19 19:17:19, benjhayden_chromium wrote:
> On 2012/06/19 18:06:53, asanka wrote:
> > You'll also want to run extract_actions.py and include the updated
> > chromeactions.txt file.
> > 
> > LGTM
> 
> Um, so I did that and it just added an unrelated action
> (BadMessageTerminate_RDH).
> So I moved chromeactions.txt out of the way, touched it to make an empty file,
> and re-ran extract_actions.py, and it "removed" a ton of stuff including
> downloads-new-ui.
> Should I commit it with removing all the other stuff, or hand-edit it to just
> remove downloads-new-ui?
> Or do we really want to remove downloads-new-ui? How is chromeactions.txt
used?

My bad. extract_actions.py is only supposed to add new entries. So removing the
flag from about_flags.cc should leave it as-is. The BadMessageTerminate_RDH
entry probably corresponds to a change where chromeactions.txt wasn't updated.

IOW, don't change chromeactions.txt.

Added rsesek who should know more about how chromeactions.txt is used WRT
about_flags.cc.

[Robert Sesek, 2012-06-20 00:20:42]

On 2012/06/19 20:06:43, asanka wrote:
> On 2012/06/19 19:17:19, benjhayden_chromium wrote:
> > On 2012/06/19 18:06:53, asanka wrote:
> > > You'll also want to run extract_actions.py and include the updated
> > > chromeactions.txt file.
> > > 
> > > LGTM
> > 
> > Um, so I did that and it just added an unrelated action
> > (BadMessageTerminate_RDH).
> > So I moved chromeactions.txt out of the way, touched it to make an empty
file,
> > and re-ran extract_actions.py, and it "removed" a ton of stuff including
> > downloads-new-ui.
> > Should I commit it with removing all the other stuff, or hand-edit it to
just
> > remove downloads-new-ui?
> > Or do we really want to remove downloads-new-ui? How is chromeactions.txt
> used?
> 
> My bad. extract_actions.py is only supposed to add new entries. So removing
the
> flag from about_flags.cc should leave it as-is. The BadMessageTerminate_RDH
> entry probably corresponds to a change where chromeactions.txt wasn't updated.
> 
> IOW, don't change chromeactions.txt.
> 
> Added rsesek who should know more about how chromeactions.txt is used WRT
> about_flags.cc.

People often forget to remove/add things to the file via the script, so that's
not too surprising. Though if you remove the ID from chromeactions.txt, UMA will
cease picking it up (it pulls this file from trunk).

[commit-bot: I haz the power, 2012-06-20 15:43:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benjhayden@chromium.org/10573012/4003

[commit-bot: I haz the power, 2012-06-20 15:43:58]

Presubmit check for 10573012-4003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/browser/ui

[sky, 2012-06-21 19:32:45]

LGTM

[commit-bot: I haz the power, 2012-06-22 16:17:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benjhayden@chromium.org/10573012/19001

[commit-bot: I haz the power, 2012-06-22 17:30:01]

Change committed as 143630