Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2319763003:
[BuildFix] Remove unnecessary include (Closed)
DescriptionRemove unnecessary include
Password manager does not use clients, while it includes
chrome_password_manager_client.h.
It caused flaky build failures on some trybots.
e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/82274
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/82281
BUG=None
Committed: https://crrev.com/9e0715144191ea74bf9208279740ee30cb423dce
Cr-Commit-Position: refs/heads/master@{#417243}
Patch Set 1 #Patch Set 2 : Address comment #Messages
Total messages: 18 (10 generated)
Description was changed from ========== Add a dependency to support autofill BUG=None ========== to ========== Add a dependency to support autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. This CL makes the dependency obvious to fix the flaky failures (generate included files before compiling password manager files.) BUG=None ==========
Description was changed from ========== Add a dependency to support autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. This CL makes the dependency obvious to fix the flaky failures (generate included files before compiling password manager files.) BUG=None ========== to ========== Add a dependency to support autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ==========
Description was changed from ========== Add a dependency to support autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ========== to ========== Add a dependency on autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ==========
Description was changed from ========== Add a dependency on autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ========== to ========== Add a dependency on autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. (introduced by https://codereview.chromium.org/2270333002) It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ==========
peria@chromium.org changed reviewers: + leon.han@intel.com, vabr@chromium.org
PTL
On 2016/09/08 04:16:47, peria wrote: > PTL Hi, thanks for the fixing ;-) But I think components/password_manager/content/public/interfaces has no dependency on components/autofill/content/public/interfaces. I checked around the build failure and I suppose that password_manager_test_base.cc should have no need to include "chrome/browser/password_manager/chrome_password_manager_client.h". Maybe we could just remove this useless including to fix this build failure. WDYT? Thanks.
Description was changed from ========== Add a dependency on autofill in password manager. Password manager does not have a dependency on auto-fill, but it includes a generated auto-fill file. (introduced by https://codereview.chromium.org/2270333002) It causes flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... This CL makes the dependency obvious to fix the flaky failures (to generate included files before compiling password manager files.) BUG=None ========== to ========== Remove unnecessary include Password manager does not use clients, while it includes chrome_password_manager_client.h. It caused flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... BUG=None ==========
Patchset #2 (id:20001) has been deleted
On 2016/09/08 06:41:01, leonhsl wrote: > On 2016/09/08 04:16:47, peria wrote: > > PTL > > Hi, thanks for the fixing ;-) > But I think components/password_manager/content/public/interfaces has no > dependency on components/autofill/content/public/interfaces. > I checked around the build failure and I suppose that > password_manager_test_base.cc should have no need to include > "chrome/browser/password_manager/chrome_password_manager_client.h". Maybe we > could just remove this useless including to fix this build failure. WDYT? > Thanks. Yes, and it should be. Updated the patch and the description. Thank you for the investigation.
lgtm as non OWNER.
On 2016/09/08 08:29:04, leonhsl wrote: > lgtm as non OWNER. LGTM. Thanks, peria@, for the patch, and Han Leon for the analysis (which I agree with). Cheers, Vaclav
The CQ bit was checked by peria@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Remove unnecessary include Password manager does not use clients, while it includes chrome_password_manager_client.h. It caused flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... BUG=None ========== to ========== Remove unnecessary include Password manager does not use clients, while it includes chrome_password_manager_client.h. It caused flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... BUG=None ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Remove unnecessary include Password manager does not use clients, while it includes chrome_password_manager_client.h. It caused flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... BUG=None ========== to ========== Remove unnecessary include Password manager does not use clients, while it includes chrome_password_manager_client.h. It caused flaky build failures on some trybots. e.g. https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds... BUG=None Committed: https://crrev.com/9e0715144191ea74bf9208279740ee30cb423dce Cr-Commit-Position: refs/heads/master@{#417243} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9e0715144191ea74bf9208279740ee30cb423dce Cr-Commit-Position: refs/heads/master@{#417243} |
