Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1789713004:
Log the location where we start to search .gclient for (Closed)
|
Created:
4 years, 9 months ago by Sergiy Byelozyorov Modified:
4 years, 9 months ago Reviewers:
Paweł Hajdan Jr. CC:
chromium-reviews, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/depot_tools.git@master Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
DescriptionLog the location where we start to search .gclient for
R=phajdan.jr@chromium.org
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299261
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix #Patch Set 3 : Fix #Messages
Total messages: 21 (8 generated)
LGTM w/optional comment https://codereview.chromium.org/1789713004/diff/1/gclient.py File gclient.py (right): https://codereview.chromium.org/1789713004/diff/1/gclient.py#newcode1383 gclient.py:1383: path = gclient_utils.FindGclientRoot(os.getcwd(), options.config_filename) While you're here, why not instrument FindGclientRoot as well? It has e.g. these "might not be the file you want to use" messages, which I think sometimes trigger for legitimate workspaces, I was never sure why. Up to you.
https://codereview.chromium.org/1789713004/diff/1/gclient.py File gclient.py (right): https://codereview.chromium.org/1789713004/diff/1/gclient.py#newcode1383 gclient.py:1383: path = gclient_utils.FindGclientRoot(os.getcwd(), options.config_filename) On 2016/03/11 22:26:13, Paweł Hajdan Jr. wrote: > While you're here, why not instrument FindGclientRoot as well? > > It has e.g. these "might not be the file you want to use" messages, which I > think sometimes trigger for legitimate workspaces, I was never sure why. > > Up to you. Not sure what to instrument inside FindGclientRoot. The message you are referring to is shown when .gclient_entities is missing and the code is not sure if the directory you are calling gclient-sync from is part of the checkout or not.
The CQ bit was checked by sergiyb@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1789713004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1789713004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Depot Tools Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Depot%20Tools%20Presubm...)
The CQ bit was checked by sergiyb@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1789713004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1789713004/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Depot Tools Presubmit on tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Depot%20Tools%20Presubm...)
I made significant changes, can you PTAL?
The CQ bit was checked by sergiyb@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1789713004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1789713004/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM
The CQ bit was checked by phajdan.jr@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1789713004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1789713004/40001
Message was sent while issue was closed.
Description was changed from ========== Log the location where we start to search .gclient for R=phajdan.jr@chromium.org ========== to ========== Log the location where we start to search .gclient for R=phajdan.jr@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299261 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299261 |
