Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
634313006:
Use the function recently added to common in svndiff.py (Closed)
DescriptionUse the function recently added to common in svndiff.py
GetModifiedFiles() function was added to common repo in
https://skia.googlesource.com/common/+/af1de867422c42855ae54ed3838c62ea445ea6b4
BUG=None
TEST=None
R=borenet@google.com
Committed: https://skia.googlesource.com/skia/+/8f1f7f821c8383053bdf747b89d843bbddd84203
Patch Set 1 #
Total comments: 3
Patch Set 2 : roll DEPS + CHECKOUT_ROOT #Messages
Total messages: 9 (2 generated)
Please, could you take a look? Thanks, https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py File tools/svndiff.py (right): https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py#newcode49 tools/svndiff.py:49: from common.py.utils import git_utils is this right? I'm not familiar with python way to import things. I copied from roll_deps.py gm/rebaseline_server/ scripts do it differently, as: from py.utils import git_utils
You'll also need to roll the common repo DEPS to pick up your change. https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py File tools/svndiff.py (right): https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py#newcode49 tools/svndiff.py:49: from common.py.utils import git_utils On 2014/10/09 03:07:10, tfarina wrote: > is this right? I'm not familiar with python way to import things. I copied from > roll_deps.py > > gm/rebaseline_server/ scripts do it differently, as: > > from py.utils import git_utils It depends on what's in PYTHONPATH. I'm fine with using "from common.py.utils", but you'll need to add the checkout root to PYTHONPATH: CHECKOUT_ROOT = os.path.realpath(os.path.join(os.path.dirname(__file__), os.pardir)) sys.path.append(CHECKOUT_ROOT) I believe that in the buildbot repo we added the common directory to PYTHONPATH (hence "from py.utils") because "common" conflicts with a directory added to PYTHONPATH by upstream code.
DEPS rolled. I used the commit sha1. Should I have used the tree sha1? I was not sure. Please, could you take another look? https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py File tools/svndiff.py (right): https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py#newcode49 tools/svndiff.py:49: from common.py.utils import git_utils On 2014/10/09 12:14:36, borenet wrote: > On 2014/10/09 03:07:10, tfarina wrote: > > is this right? I'm not familiar with python way to import things. I copied > from > > roll_deps.py > > > > gm/rebaseline_server/ scripts do it differently, as: > > > > from py.utils import git_utils > > It depends on what's in PYTHONPATH. I'm fine with using "from common.py.utils", > but you'll need to add the checkout root to PYTHONPATH: > > CHECKOUT_ROOT = os.path.realpath(os.path.join(os.path.dirname(__file__), > os.pardir)) > sys.path.append(CHECKOUT_ROOT) > > I believe that in the buildbot repo we added the common directory to PYTHONPATH > (hence "from py.utils") because "common" conflicts with a directory added to > PYTHONPATH by upstream code. Done.
On 2014/10/10 00:03:59, tfarina wrote: > DEPS rolled. I used the commit sha1. Should I have used the tree sha1? I was not > sure. > > Please, could you take another look? > > https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py > File tools/svndiff.py (right): > > https://codereview.chromium.org/634313006/diff/1/tools/svndiff.py#newcode49 > tools/svndiff.py:49: from common.py.utils import git_utils > On 2014/10/09 12:14:36, borenet wrote: > > On 2014/10/09 03:07:10, tfarina wrote: > > > is this right? I'm not familiar with python way to import things. I copied > > from > > > roll_deps.py > > > > > > gm/rebaseline_server/ scripts do it differently, as: > > > > > > from py.utils import git_utils > > > > It depends on what's in PYTHONPATH. I'm fine with using "from > common.py.utils", > > but you'll need to add the checkout root to PYTHONPATH: > > > > CHECKOUT_ROOT = os.path.realpath(os.path.join(os.path.dirname(__file__), > > os.pardir)) > > sys.path.append(CHECKOUT_ROOT) > > > > I believe that in the buildbot repo we added the common directory to > PYTHONPATH > > (hence "from py.utils") because "common" conflicts with a directory added to > > PYTHONPATH by upstream code. > > Done. Change LGTM. Now that I think about it though, I'm not really sure what this file is being used for, if anything. I'm fine with committing this now, but it's probably time we went through and cleaned up the no-longer-used scripts.
The CQ bit was checked by tfarina@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/634313006/20001
The CQ bit was unchecked by commit-bot@chromium.org
Presubmit check for 634313006-20001 failed and returned exit status 1. Running presubmit commit checks ... ** Presubmit ERRORS ** Pylint (tools/svndiff.py) (1.06s) failed ************* Module /b/infra_internal/commit_queue/workdir/skia/tools/svndiff.py W0311: 44,0: Bad indentation. Found 4 spaces, expected 2 W0311: 74,0: Bad indentation. Found 4 spaces, expected 2 W0311: 78,0: Bad indentation. Found 4 spaces, expected 2 W0311: 79,0: Bad indentation. Found 4 spaces, expected 2 W0311: 80,0: Bad indentation. Found 4 spaces, expected 2 W0311: 81,0: Bad indentation. Found 8 spaces, expected 4 W0311: 84,0: Bad indentation. Found 4 spaces, expected 2 W0311: 90,0: Bad indentation. Found 4 spaces, expected 2 W0311: 91,0: Bad indentation. Found 8 spaces, expected 4 W0311: 92,0: Bad indentation. Found 12 spaces, expected 6 W0311: 93,0: Bad indentation. Found 8 spaces, expected 4 W0311: 95,0: Bad indentation. Found 4 spaces, expected 2 W0311: 97,0: Bad indentation. Found 4 spaces, expected 2 W0311: 98,0: Bad indentation. Found 4 spaces, expected 2 W0311: 99,0: Bad indentation. Found 8 spaces, expected 4 W0311:101,0: Bad indentation. Found 4 spaces, expected 2 W0311:105,0: Bad indentation. Found 4 spaces, expected 2 W0311:106,0: Bad indentation. Found 8 spaces, expected 4 W0311:107,0: Bad indentation. Found 12 spaces, expected 6 W0311:108,0: Bad indentation. Found 4 spaces, expected 2 W0311:113,0: Bad indentation. Found 4 spaces, expected 2 W0311:115,0: Bad indentation. Found 4 spaces, expected 2 W0311:116,0: Bad indentation. Found 8 spaces, expected 4 W0311:117,0: Bad indentation. Found 8 spaces, expected 4 W0311:118,0: Bad indentation. Found 8 spaces, expected 4 W0311:119,0: Bad indentation. Found 8 spaces, expected 4 W0311:120,0: Bad indentation. Found 4 spaces, expected 2 W0311:121,0: Bad indentation. Found 8 spaces, expected 4 W0311:126,0: Bad indentation. Found 4 spaces, expected 2 W0311:134,0: Bad indentation. Found 4 spaces, expected 2 W0311:142,0: Bad indentation. Found 4 spaces, expected 2 W0311:149,0: Bad indentation. Found 4 spaces, expected 2 W0311:150,0: Bad indentation. Found 4 spaces, expected 2 W0311:152,0: Bad indentation. Found 4 spaces, expected 2 W0311:153,0: Bad indentation. Found 4 spaces, expected 2 W0311:157,0: Bad indentation. Found 8 spaces, expected 4 W0311:158,0: Bad indentation. Found 8 spaces, expected 4 W0311:159,0: Bad indentation. Found 12 spaces, expected 6 W0311:163,0: Bad indentation. Found 12 spaces, expected 6 W0311:168,0: Bad indentation. Found 8 spaces, expected 4 W0311:169,0: Bad indentation. Found 8 spaces, expected 4 W0311:170,0: Bad indentation. Found 12 spaces, expected 6 W0311:174,0: Bad indentation. Found 12 spaces, expected 6 W0311:180,0: Bad indentation. Found 4 spaces, expected 2 W0311:185,0: Bad indentation. Found 4 spaces, expected 2 W0311:188,0: Bad indentation. Found 4 spaces, expected 2 W0311:189,0: Bad indentation. Found 4 spaces, expected 2 W0311:190,0: Bad indentation. Found 8 spaces, expected 4 W0311:191,0: Bad indentation. Found 4 spaces, expected 2 W0311:195,0: Bad indentation. Found 4 spaces, expected 2 W0311:214,0: Bad indentation. Found 4 spaces, expected 2 W0311:215,0: Bad indentation. Found 4 spaces, expected 2 W0311:216,0: Bad indentation. Found 8 spaces, expected 4 W0311:217,0: Bad indentation. Found 8 spaces, expected 4 W0311:218,0: Bad indentation. Found 8 spaces, expected 4 W0311:219,0: Bad indentation. Found 12 spaces, expected 6 W0311:222,0: Bad indentation. Found 4 spaces, expected 2 W0311:229,0: Bad indentation. Found 4 spaces, expected 2 W0311:230,0: Bad indentation. Found 4 spaces, expected 2 W0311:231,0: Bad indentation. Found 8 spaces, expected 4 W0311:232,0: Bad indentation. Found 4 spaces, expected 2 W0311:234,0: Bad indentation. Found 4 spaces, expected 2 W0311:235,0: Bad indentation. Found 4 spaces, expected 2 W0311:236,0: Bad indentation. Found 4 spaces, expected 2 W0311:237,0: Bad indentation. Found 4 spaces, expected 2 W0311:238,0: Bad indentation. Found 6 spaces, expected 4 W0311:239,0: Bad indentation. Found 4 spaces, expected 2 W0311:240,0: Bad indentation. Found 6 spaces, expected 4 W0311:243,0: Bad indentation. Found 4 spaces, expected 2 W0311:244,0: Bad indentation. Found 4 spaces, expected 2 W0311:245,0: Bad indentation. Found 4 spaces, expected 2 W0311:246,0: Bad indentation. Found 4 spaces, expected 2 W0311:247,0: Bad indentation. Found 8 spaces, expected 4 W0311:248,0: Bad indentation. Found 8 spaces, expected 4 W0311:252,0: Bad indentation. Found 4 spaces, expected 2 W0311:253,0: Bad indentation. Found 8 spaces, expected 4 W0311:255,0: Bad indentation. Found 4 spaces, expected 2 W0311:256,0: Bad indentation. Found 8 spaces, expected 4 W0311:261,0: Bad indentation. Found 4 spaces, expected 2 W0311:262,0: Bad indentation. Found 8 spaces, expected 4 W0311:265,0: Bad indentation. Found 12 spaces, expected 6 W0311:266,0: Bad indentation. Found 12 spaces, expected 6 W0311:267,0: Bad indentation. Found 12 spaces, expected 6 W0311:268,0: Bad indentation. Found 16 spaces, expected 8 W0311:270,0: Bad indentation. Found 12 spaces, expected 6 W0311:271,0: Bad indentation. Found 16 spaces, expected 8 W0311:273,0: Bad indentation. Found 12 spaces, expected 6 W0311:274,0: Bad indentation. Found 12 spaces, expected 6 W0311:277,0: Bad indentation. Found 12 spaces, expected 6 W0311:282,0: Bad indentation. Found 12 spaces, expected 6 W0311:283,0: Bad indentation. Found 8 spaces, expected 4 W0311:284,0: Bad indentation. Found 12 spaces, expected 6 W0311:286,0: Bad indentation. Found 12 spaces, expected 6 W0311:287,0: Bad indentation. Found 16 spaces, expected 8 W0311:290,0: Bad indentation. Found 12 spaces, expected 6 W0311:291,0: Bad indentation. Found 16 spaces, expected 8 W0311:294,0: Bad indentation. Found 12 spaces, expected 6 W0311:295,0: Bad indentation. Found 16 spaces, expected 8 W0311:300,0: Bad indentation. Found 4 spaces, expected 2 W0311:302,0: Bad indentation. Found 4 spaces, expected 2 W0311:305,0: Bad indentation. Found 4 spaces, expected 2 W0311:309,0: Bad indentation. Found 4 spaces, expected 2 W0311:311,0: Bad indentation. Found 4 spaces, expected 2 W0311:312,0: Bad indentation. Found 4 spaces, expected 2 W0311:313,0: Bad indentation. Found 8 spaces, expected 4 W0311:314,0: Bad indentation. Found 4 spaces, expected 2 W0311:318,0: Bad indentation. Found 4 spaces, expected 2 W0311:319,0: Bad indentation. Found 4 spaces, expected 2 W0311:324,0: Bad indentation. Found 4 spaces, expected 2 W0311:329,0: Bad indentation. Found 4 spaces, expected 2 W0311:334,0: Bad indentation. Found 4 spaces, expected 2 W0311:335,0: Bad indentation. Found 4 spaces, expected 2 W0105: 17,-1: String statement has no effect W0621:179,16:_RunCommand: Redefining name 'args' from outer scope (line 334) W0621:214,4:_GitExportBaseVersionOfFile: Redefining name 'args' from outer scope (line 334) W0622:246,8:SvnDiff: Redefining built-in 'dir' W0702:239,4:SvnDiff: No exception type(s) specified W0621:308,18:Main: Redefining name 'args' from outer scope (line 334) W0621:308,9:Main: Redefining name 'options' from outer scope (line 334) Presubmit checks took 4.2s to calculate.
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 8f1f7f821c8383053bdf747b89d843bbddd84203. |
