Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(465)

Issue 472863002: Remove lib32 code from install-build-deps.sh (Closed)

Created:
6 years, 4 months ago by Paweł Hajdan Jr.
Modified:
6 years, 4 months ago
Reviewers:
Michael Moss
CC:
chromium-reviews
Project:
chromium
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -216 lines) Patch
M build/install-build-deps.sh View 4 chunks +2 lines, -216 lines 3 comments Download

Messages

Total messages: 5 (0 generated)
Paweł Hajdan Jr.
6 years, 4 months ago (2014-08-14 14:24:43 UTC) #1
Michael Moss
lgtm https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh File build/install-build-deps.sh (right): https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh#newcode47 build/install-build-deps.sh:47: # TODO(phajdan.jr): Remove the lib32 flags when nothing ...
6 years, 4 months ago (2014-08-14 14:59:14 UTC) #2
Paweł Hajdan Jr.
https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh File build/install-build-deps.sh (right): https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh#newcode47 build/install-build-deps.sh:47: # TODO(phajdan.jr): Remove the lib32 flags when nothing else ...
6 years, 4 months ago (2014-08-18 11:49:46 UTC) #3
Paweł Hajdan Jr.
Committed patchset #1 manually as 290235 (presubmit successful).
6 years, 4 months ago (2014-08-18 12:30:28 UTC) #4
Michael Moss
6 years, 4 months ago (2014-08-18 15:53:21 UTC) #5
Message was sent while issue was closed.
https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh
File build/install-build-deps.sh (right):

https://codereview.chromium.org/472863002/diff/1/build/install-build-deps.sh#...
build/install-build-deps.sh:47: # TODO(phajdan.jr): Remove the lib32 flags when
nothing else refers to them.
On 2014/08/18 11:49:45, Paweł Hajdan Jr. wrote:
> On 2014/08/14 14:59:14, Michael Moss wrote:
> > I'd almost like to see it fail here, so we can find and remove uses more
> > quickly. Can it least present a big "OBSOLETE!!!!" warning or something, so
> > maybe people still using the flag will notice and stop?
> 
> That's why I sent a chromium-dev e-mail.
> 
> I think a warning even in all caps is easy to miss, and waiting for user input
> could break non-interactive workflows.
> 
> If this doesn't install the right thing, I'm sure people *will* notice, with
or
> without the warning.
> 
> I got a positive confirmation in that thread that this flag should no longer
be
> needed.

Hence my first sentence where I said I'd like to see it just fail (i.e. take it
out and let it generate a usage error). Why keep accepting it, which implies
it's still doing something?

Powered by Google App Engine
This is Rietveld 408576698