[telemetry] Refactoring script.

[telemetry] Refactoring script.

- For refactor v0, add the ability to move modules but not rename them.
- Merge count script into unified refactor script that handles all our ghetto grepping needs.

Also:
- Fix InDirectory and move it out of find_dependencies to telemetry.util.path.
- Change find_dependencies to use telemetry.util.path.

BUG=449308
TEST=tools/telemetry$ mkdir telemetry/internal; ./refactor mv telemetry/core/ telemetry/internal/;  # To clean up, do "rm -r telemetry/internal/core && git reset --hard"

Committed: https://crrev.com/4427329580ef6a3ecfe97fc33c440e043ce3bec2
Cr-Commit-Position: refs/heads/master@{#323064}

[dtu, 2015-03-26 18:09:43]

Patchset #1 (id:1) has been deleted

[dtu, 2015-03-26 18:11:06]

dtu@chromium.org changed reviewers:
+ aiolos@chromium.org, eakuefner@chromium.org

[eakuefner, 2015-03-27 17:55:06]

This is sweet!

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refactor
File tools/telemetry/refactor (right):

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:30: os.path.join(path.GetChromiumSrcDir(),
'third_party', 'trace-viewer',
I'm a little worried about potentially accidentally mangling stuff in
trace-viewer by including it here, but I suppose we can always just check it out
again. I do like the idea of counting stuff in trace-viewer as well.

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:57: """Apply a transformation to each group of imports
in the given module."""
Please write the expected type of `transformation`. You don't have to go full
Google-style "Args:/Returns:" and so forth since this is a fairly short-term and
private script, but it would help me read the code.

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:273: class
RefactorCommand(command_line.SubcommandCommand):
Do we get usage text for free with this?

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/telemet...
File tools/telemetry/telemetry/util/path.py (right):

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/telemet...
tools/telemetry/telemetry/util/path.py:49: A little bit of a misnomer, since it
works even if they're files."""
Do we actually need this? Why not just get the paths as strings and use
string.startswith?

If you're going to keep this I'd say remove this disclaimer.

Either SubpathOf(subpath, superpath) and then it will obvious that it can be
used for either, or keep the signature as-is; it's awkward if I call InDirectory
with subdirectory == something that isn't actually a directory.

[dtu, 2015-03-27 19:17:15]

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refactor
File tools/telemetry/refactor (right):

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:30: os.path.join(path.GetChromiumSrcDir(),
'third_party', 'trace-viewer',
On 2015/03/27 17:55:05, eakuefner wrote:
> I'm a little worried about potentially accidentally mangling stuff in
> trace-viewer by including it here, but I suppose we can always just check it
out
> again. I do like the idea of counting stuff in trace-viewer as well.

Since we plan on using Telemetry in the future, it's better to keep the
directory in there. We should only be modifying files that have Telemetry
dependencies anyway.

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:57: """Apply a transformation to each group of imports
in the given module."""
On 2015/03/27 17:55:06, eakuefner wrote:
> Please write the expected type of `transformation`. You don't have to go full
> Google-style "Args:/Returns:" and so forth since this is a fairly short-term
and
> private script, but it would help me read the code.

I did it anyway, since the function is non-trivial.

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/refacto...
tools/telemetry/refactor:273: class
RefactorCommand(command_line.SubcommandCommand):
On 2015/03/27 17:55:05, eakuefner wrote:
> Do we get usage text for free with this?

Yes, but its default formatting is like so-so.

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/telemet...
File tools/telemetry/telemetry/util/path.py (right):

https://codereview.chromium.org/1033053002/diff/20001/tools/telemetry/telemet...
tools/telemetry/telemetry/util/path.py:49: A little bit of a misnomer, since it
works even if they're files."""
On 2015/03/27 17:55:06, eakuefner wrote:
> Do we actually need this? Why not just get the paths as strings and use
> string.startswith?
> 
> If you're going to keep this I'd say remove this disclaimer.
> 
> Either SubpathOf(subpath, superpath) and then it will obvious that it can be
> used for either, or keep the signature as-is; it's awkward if I call
InDirectory
> with subdirectory == something that isn't actually a directory.

This is surprisingly tricky, and people get it wrong all the time. Using
startswith() or os.path.commonprefix() will fail if one directory is a substring
of another. Here's an example:
'foo/bar_test/nom.py'.startswith('foo/bar') is True, but we want False.

(My original version of this function in find_dependencies was incorrect.)

Did the rename, though.

[eakuefner, 2015-03-27 20:14:34]

lgtm

[aiolos (Not reviewing), 2015-03-31 02:50:34]

lgtm

[dtu, 2015-03-31 15:59:43]

The CQ bit was checked by dtu@chromium.org

[dtu, 2015-03-31 15:59:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
eakuefner@chromium.org
Link to the patchset: https://codereview.chromium.org/1033053002/#ps60001
(title: "Comment fixup")

[commit-bot: I haz the power, 2015-03-31 15:59:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1033053002/60001

[commit-bot: I haz the power, 2015-03-31 17:24:55]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-03-31 17:25:46]

Patchset 3 (id:??) landed as
https://crrev.com/4427329580ef6a3ecfe97fc33c440e043ce3bec2
Cr-Commit-Position: refs/heads/master@{#323064}