In WPT importer, abort if there are exportable-but-not-yet-exported commits.

In WPT importer, abort if there are exportable-but-not-yet-exported commits.

In this CL:
 - In importer main method, check for exportable but not yet exported commits, and if there are any, print an error message and abort.
 - In order to put this in the main method, this CL also changes it so that checking out the repo is done in the main method.
 - There are also a couple places where variables are renamed, in order to try to improve clarity.

BUG=666540
Review-Url: https://codereview.chromium.org/2655183002
Cr-Commit-Position: refs/heads/master@{#446579}
Committed: https://chromium.googlesource.com/chromium/src/+/15da4792dd9356efe3ce47f101956bcfe6f3638b

[qyearsley, 2017-01-26 00:03:05]

Description was changed from

==========
In WPT importer, abort if there are exportable-but-not-yet-exported commits.

BUG=666540
==========

to

==========
In WPT importer, abort if there are exportable-but-not-yet-exported commits.

In this CL:
 - In importer main method, check for exportable but not yet exported commits,
and if there are any, print an error message and abort.
 - In order to put this in the main method, this CL also changes it so that
checking out the repo is done in the main method.
 - There are also a couple places where variables are renamed, in order to try
to improve clarity.

BUG=666540
==========

[qyearsley, 2017-01-26 00:03:05]

qyearsley@chromium.org changed reviewers:
+ jeffcarp@chromium.org

[qyearsley, 2017-01-26 00:04:34]

Does this look like it makes sense?

As-is, this uses the previous existing logic for checking out the repo (not the
fetch method in LocalWPT), and then looks at commits in that existing local WPT
repo.

This could be changed and simplified once csswg-test is merged into WPT...

[jeffcarp, 2017-01-26 00:17:15]

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:72:
self.run(['git', 'clone', repo_url, temp_repo_path])
It would be nice to deduplicate the cloning code. Could LocalWPT.fetch be used
here? (that would also require LocalWPT to be instantiated above and be passed
into exportable_but_not_exported_commits)

[qyearsley, 2017-01-26 00:30:07]

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:72:
self.run(['git', 'clone', repo_url, temp_repo_path])
On 2017/01/26 at 00:17:15, jeffcarp wrote:
> It would be nice to deduplicate the cloning code. Could LocalWPT.fetch be used
here? (that would also require LocalWPT to be instantiated above and be passed
into exportable_but_not_exported_commits)

That would be possible, although this code is also used to update the csswg-test
repo now, and currently the LocalWPT.fetch would only fetch the
web-platform-tests repo. It might be confusing to change LocalWPT to take a repo
URL and also handle csswg-test if passed the URL... anyway maybe I should add a
comment here noting that we want to simplify this after csswg-test is merged
into web-platform-tests?

[qyearsley, 2017-01-26 18:32:33]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 18:33:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 20:18:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 20:18:17]

Dry run: This issue passed the CQ dry run.

[jeffcarp, 2017-01-27 02:05:41]

lgtm

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):

https://codereview.chromium.org/2655183002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:72:
self.run(['git', 'clone', repo_url, temp_repo_path])
On 2017/01/26 at 00:30:07, qyearsley wrote:
> On 2017/01/26 at 00:17:15, jeffcarp wrote:
> > It would be nice to deduplicate the cloning code. Could LocalWPT.fetch be
used here? (that would also require LocalWPT to be instantiated above and be
passed into exportable_but_not_exported_commits)
> 
> That would be possible, although this code is also used to update the
csswg-test repo now, and currently the LocalWPT.fetch would only fetch the
web-platform-tests repo. It might be confusing to change LocalWPT to take a repo
URL and also handle csswg-test if passed the URL... anyway maybe I should add a
comment here noting that we want to simplify this after csswg-test is merged
into web-platform-tests?

Excellent, sgtm

[qyearsley, 2017-01-27 03:40:28]

The CQ bit was checked by qyearsley@chromium.org

[commit-bot: I haz the power, 2017-01-27 03:41:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 03:46:04]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485488428473680,
"parent_rev": "fb6e9d1fd67d2ee811d5276e6d07940cb2faf342", "commit_rev":
"15da4792dd9356efe3ce47f101956bcfe6f3638b"}

[commit-bot: I haz the power, 2017-01-27 03:47:20]

Description was changed from

==========
In WPT importer, abort if there are exportable-but-not-yet-exported commits.

In this CL:
 - In importer main method, check for exportable but not yet exported commits,
and if there are any, print an error message and abort.
 - In order to put this in the main method, this CL also changes it so that
checking out the repo is done in the main method.
 - There are also a couple places where variables are renamed, in order to try
to improve clarity.

BUG=666540
==========

to

==========
In WPT importer, abort if there are exportable-but-not-yet-exported commits.

In this CL:
 - In importer main method, check for exportable but not yet exported commits,
and if there are any, print an error message and abort.
 - In order to put this in the main method, this CL also changes it so that
checking out the repo is done in the main method.
 - There are also a couple places where variables are renamed, in order to try
to improve clarity.

BUG=666540

Review-Url: https://codereview.chromium.org/2655183002
Cr-Commit-Position: refs/heads/master@{#446579}
Committed:
https://chromium.googlesource.com/chromium/src/+/15da4792dd9356efe3ce47f10195...
==========

[commit-bot: I haz the power, 2017-01-27 03:47:25]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/15da4792dd9356efe3ce47f10195...