Package tidy #10590

edunham · 2016-04-13T23:34:52Z

This fixes #861.

@askeing, I've copied your work from https://github.com/askeing/servo_tidy and attributed the commit to you. My commit in this PR is Git housekeeping to preserve tidy's history. If you'd like to make additional changes, I've given you and @shinglyu push access to my fork of Servo. Apologies if this is already familiar, but the workflow for pushing to my branch is:

# in your clone of the Servo repo
$ git remote add edunham git@github.com:edunham/servo.git
$ git checkout -b package-tidy
$ git pull edunham package-tidy
# make commits that you'd like to see in this PR
$ git push edunham package-tidy

Once this lands, I'll look at how to publish it to PyPI and automate that process.

Please don't merge this yet; we still need to discuss how the change should work around https://github.com/servo/servo/blob/master/python/servo/testing_commands.py#L33 , as I've yet to figure out how to get the egg to actually expose its tests.

This change is


        start Tidy move


        Create a Python package of Tidy


        add tidy to README

highfive · 2016-04-13T23:34:57Z

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

highfive · 2016-04-13T23:34:58Z

Heads up! This PR modifies the following files:

@wafflespeanut: python/tidy.py, python/tidy/tests/init.py, python/tidy_self_test/tidy_self_test.py, python/README.md, python/tidy/HISTORY.rst, python/tidy/servo_tidy/licenseck.py, python/tidy/setup.py, python/tidy_self_test/init.py, python/tidy_self_test/rust_tidy.rs, python/tidy_self_test/wrong_space.rs, python/tidy/servo_tidy/init.py, python/tidy_self_test/long_line.rs, python/tidy/tests/test_tidy.py, python/tidy/tests/test.toml, python/tidy_self_test/whatwg_link.rs, python/tidy/tests/wrong_space.rs, python/tidy/tests/long_line.rs, python/tidy/tests/rust_tidy.rs, python/tidy/README.rst, python/tidy/tests/tidy_self_test.py, python/tidy_self_test/spec.webidl, python/tidy/tests/whatwg_link.rs, python/tidy/Makefile, python/tidy/tests/spec.webidl, python/tidy_self_test/incorrect_license.rs, python/tidy/tests/incorrect_license.rs, python/tidy_self_test/speclink.rs, python/tidy/servo_tidy/tidy.py, python/tidy/tests/speclink.rs, python/tidy_self_test/test.toml

edunham · 2016-04-13T23:37:14Z

Do we know whether anyone's actually running the Tidy selftests through the mach-centric testing_commands.py wrapper? If that's not a priority, removing that functionality would be the easiest workaround.

jdm · 2016-04-14T01:16:27Z

I'm not entirely clear what you're asking; TravisCI is running ./mach test-tidy --self-test which corresponds to that block in testing_commands.py, I believe.


        Merge branch 'package-tidy' of github.com:edunham/servo into package-…

…tidy


        Package tidy

- Modified the testing commands - Added the requirements - Moved python/tidy/tests to python/tidy/servo_tidy_tests for self tidy tests

askeing · 2016-04-14T08:03:35Z

Due to servo_tidy cannot be found on PyPI now, the test will failed.
Here's the local test steps:

$ cd <SERVO>
$ rm -rf ./python/_virtualenv/
$ ./mach help
$ # for init the _virtualenv, it will show error msg: Could not find a version that satisfies the requirement servo-tidy==0.0.1
$ 
$ source ./python/_virtualenv/bin/activate
$ pip install -e python/tidy/
$ # install servo_tidy from local
$ deactivate
$ 
$ ./mach test-tidy --self-test


        Modify the description of setup.py


        Modified the servo_tidy_tests for both "./mach" and "make test"

- fix `cd python/tidy; make test` fail issue


        Use local `tidy`

#861 (comment) "I think the most important concern is that it's possible to modify tidy.py and see how those changes affect ./mach test-tidy with the fewest possible intermediate steps." - jdm This takes publishing complexity away from the contributor when testing changes and makes it an infra problem instead, where it's much easier to automate & saner to manage pypi credentials

askeing · 2016-04-14T17:19:23Z

python/requirements.txt

@@ -13,3 +13,5 @@ pyflakes == 0.8.0

 # For test-webidl
 ply == 3.8
+
+-e python/tidy


Cool!
I never knew we can use -e foo/bar in requiremnets.txt before.
It's really helpful to avoid the publish problems, and make testing easier :)


        Add servo_tidy_tests into ignore dirs, and modify setup.py to fit the…

… tidy check - fix the issue of '$ ./mach test-tidy --no-progress'

askeing · 2016-04-15T06:46:29Z

All checks have passed :)
Do I have to rebase the commits before review? Or after get the r+?

edunham · 2016-04-15T16:17:15Z

@askeing we'll only need to rebase if the master branch develops a merge conflict with these changes before the PR lands. Since there've been no major changes in tidy or the other files touched by this PR since the package-tidy branch was created, we don't have to worry about rebasing. Our next steps are to get the code reviewed, wait for bors-servo to run the tests one more time after the r+, and then watch it automatically land if nobody has landed anything conflicting in the meantime!

@bors-servo r? @larsbergstrom

larsbergstrom · 2016-04-15T17:21:42Z

Reviewed 1 of 43 files at r1, 1 of 29 files at r2, 10 of 42 files at r4, 28 of 33 files at r5, 2 of 3 files at r6, 1 of 1 files at r7, 2 of 2 files at r8.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

larsbergstrom · 2016-04-15T17:22:01Z

@bors-servo r+

bors-servo · 2016-04-15T17:22:05Z

📌 Commit ee433a0 has been approved by larsbergstrom

bors-servo · 2016-04-15T17:50:17Z

⌛ Testing commit ee433a0 with merge bfe5453...


        Auto merge of #10590 - edunham:package-tidy, r=larsbergstrom

@askeing

Package tidy This fixes #861. @askeing, I've copied your work from https://github.com/askeing/servo_tidy and attributed the commit to you. My commit in this PR is Git housekeeping to preserve `tidy`'s history. If you'd like to make additional changes, I've given you and @shinglyu push access to my fork of Servo. Apologies if this is already familiar, but the workflow for pushing to my branch is: ``` $ git remote add edunham git@github.com:edunham/servo.git $ git checkout -b package-tidy $ git pull edunham package-tidy $ git push edunham package-tidy ``` Once this lands, I'll look at how to publish it to PyPI and automate that process. Please don't merge this yet; we still need to discuss how the change should work around https://github.com/servo/servo/blob/master/python/servo/testing_commands.py#L33 , as I've yet to figure out how to get the egg to actually expose its tests.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10590)

bors-servo · 2016-04-15T18:31:22Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt


        Auto merge of #10635 - edunham:tidy-cleanup, r=larsbergstrom

@larsbergstrom

Docs and cleanup after moving tidy Missed a couple things in #10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? @larsbergstrom (sorry for PR spamming today)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10635)


        Auto merge of #10635 - edunham:tidy-cleanup, r=larsbergstrom

@larsbergstrom

Docs and cleanup after moving tidy Missed a couple things in #10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? @larsbergstrom (sorry for PR spamming today)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10635)


        Auto merge of #10635 - edunham:tidy-cleanup, r=larsbergstrom

@larsbergstrom

Docs and cleanup after moving tidy Missed a couple things in #10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? @larsbergstrom (sorry for PR spamming today)  --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10635)


        servo: Merge #10635 - Docs and cleanup after moving tidy (from edunha…

…m:tidy-cleanup); r=larsbergstrom Missed a couple things in servo/servo#10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? larsbergstrom (sorry for PR spamming today) Source-Repo: https://github.com/servo/servo Source-Revision: 9d23d7c0d98e24995243487b55abbf2acd1ec354 UltraBlame original commit: 5b7a48485a8a5825e6041fccfbf7f0e69be725a3


        servo: Merge #10635 - Docs and cleanup after moving tidy (from edunha…

…m:tidy-cleanup); r=larsbergstrom Missed a couple things in servo/servo#10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? larsbergstrom (sorry for PR spamming today) Source-Repo: https://github.com/servo/servo Source-Revision: 9d23d7c0d98e24995243487b55abbf2acd1ec354 UltraBlame original commit: 5b7a48485a8a5825e6041fccfbf7f0e69be725a3


        servo: Merge #10635 - Docs and cleanup after moving tidy (from edunha…

…m:tidy-cleanup); r=larsbergstrom Missed a couple things in servo/servo#10590 `licenseck.py` was moved into `tidy/servo_tidy/licenseck.py` by the prior PR r? larsbergstrom (sorry for PR spamming today) Source-Repo: https://github.com/servo/servo Source-Revision: 9d23d7c0d98e24995243487b55abbf2acd1ec354 UltraBlame original commit: 5b7a48485a8a5825e6041fccfbf7f0e69be725a3

edunham and others added 3 commits Apr 13, 2016

start Tidy move

711bfe2

Create a Python package of Tidy

af2ca1f

add tidy to README

Loading status checks…

cd4cabf

edunham added the A-infrastructure label Apr 13, 2016

highfive assigned Manishearth Apr 13, 2016

highfive added the S-awaiting-review label Apr 13, 2016

askeing added 2 commits Apr 14, 2016

Merge branch 'package-tidy' of github.com:edunham/servo into package-…

41b8687

…tidy

Package tidy

Loading status checks…

bfe460d

- Modified the testing commands - Added the requirements - Moved python/tidy/tests to python/tidy/servo_tidy_tests for self tidy tests

askeing added 2 commits Apr 14, 2016

Modify the description of setup.py

753db5e

Modified the servo_tidy_tests for both "./mach" and "make test"

Loading status checks…

8c4c899

- fix `cd python/tidy; make test` fail issue

askeing force-pushed the edunham:package-tidy branch from c15beb4 to 8c4c899 Apr 14, 2016

edunham force-pushed the edunham:package-tidy branch from 81d0409 to e20def0 Apr 14, 2016

askeing reviewed Apr 14, 2016
View changes

Add servo_tidy_tests into ignore dirs, and modify setup.py to fit the…

Loading status checks…

ee433a0

… tidy check - fix the issue of '$ ./mach test-tidy --no-progress'

highfive assigned larsbergstrom and unassigned Manishearth Apr 15, 2016

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 15, 2016

bors-servo merged commit ee433a0 into servo:master Apr 15, 2016
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Apr 15, 2016

edunham mentioned this pull request Apr 15, 2016

Docs and cleanup after moving tidy #10635

Merged

servo / servo

Package tidy #10590

Package tidy #10590

edunham commented Apr 13, 2016

highfive commented Apr 13, 2016

highfive commented Apr 13, 2016

edunham commented Apr 13, 2016

jdm commented Apr 14, 2016

askeing commented Apr 14, 2016

This comment has been minimized.

askeing commented Apr 15, 2016

edunham commented Apr 15, 2016

larsbergstrom commented Apr 15, 2016

larsbergstrom commented Apr 15, 2016

bors-servo commented Apr 15, 2016

bors-servo commented Apr 15, 2016

bors-servo commented Apr 15, 2016

servo / servo

Join GitHub today

Package tidy #10590

Package tidy #10590

Conversation

edunham commented Apr 13, 2016

highfive commented Apr 13, 2016

highfive commented Apr 13, 2016

edunham commented Apr 13, 2016

jdm commented Apr 14, 2016

askeing commented Apr 14, 2016

This comment has been minimized.

askeing Apr 14, 2016

askeing commented Apr 15, 2016

edunham commented Apr 15, 2016

larsbergstrom commented Apr 15, 2016

larsbergstrom commented Apr 15, 2016

bors-servo commented Apr 15, 2016

bors-servo commented Apr 15, 2016

bors-servo commented Apr 15, 2016