Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'use statements with extraneous spaces' tidy check #15690

Merged
merged 1 commit into from Feb 23, 2017

Conversation

@kimumetal
Copy link
Contributor

kimumetal commented Feb 22, 2017

Add 'use statements with extraneous spaces' tidy check

I added simple check routine for 'use statements with extraneous
spaces' and codes that breaks the check routine in rust_tidy.rs.

  • Added a code that using 'use statements with extraneous spaces' code
    in rust_tidy.rs
  • Added assertion code in test_tidy.py.
  • check_rust function in tidy.py now recognizes the simple case in
    the 'use statements with extraneous spaces'.
  • Ran tidy check on rust code and modified a
    code(tests/unit/style/parsing/inherited_text.rs) that is not passing
    on this new tidy check.

TODO: this code has to be refactored to support more general cases, such as tab or newline.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #14898 (github issue number if applicable).
  • These changes do not require tests because ./mach test-tidy itself is the test for the code.

This change is Reviewable

@highfive
Copy link

highfive commented Feb 22, 2017

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/tidy/servo_tidy_tests/test_tidy.py, python/tidy/servo_tidy_tests/rust_tidy.rs, python/tidy/servo_tidy/tidy.py
@kimumetal kimumetal force-pushed the kimumetal:issue_14898 branch from 9e55037 to 280c73f Feb 22, 2017
Add 'use statements with extraneous spaces' tidy check

I added simple check routine for 'use statements with extraneous
spaces' and codes that breaks the check routine in rust_tidy.rs.

* Added a code that using 'use statements with extraneous spaces' code
  in rust_tidy.rs
* Added assertion code in test_tidy.py.
* check_rust function in tidy.py now recognizes the simple case in
  the 'use statements with extraneous spaces'.
* Ran tidy check on rust code and modified a
  code(tests/unit/style/parsing/inherited_text.rs) that is not passing
  on this new tidy check.

TODO: this code has to be refactored to support more general cases.

- [X] ./mach test-tidy does not report any errors
- [X] These changes fix #14898 (github issue number if applicable).
@kimumetal kimumetal force-pushed the kimumetal:issue_14898 branch from 280c73f to ccb14ab Feb 22, 2017
@wafflespeanut
Copy link
Member

wafflespeanut commented Feb 23, 2017

Looks great! Thanks :)

@wafflespeanut
Copy link
Member

wafflespeanut commented Feb 23, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2017

📌 Commit ccb14ab has been approved by Wafflespeanut

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2017

Testing commit ccb14ab with merge b34fdf6...

bors-servo added a commit that referenced this pull request Feb 23, 2017
Add 'use statements with extraneous spaces' tidy check

Add 'use statements with extraneous spaces' tidy check

I added simple check routine for 'use statements with extraneous
spaces' and codes that breaks the check routine in rust_tidy.rs.

* Added a code that using 'use statements with extraneous spaces' code
  in rust_tidy.rs
* Added assertion code in test_tidy.py.
* check_rust function in tidy.py now recognizes the simple case in
  the 'use statements with extraneous spaces'.
* Ran tidy check on rust code and modified a
  code(tests/unit/style/parsing/inherited_text.rs) that is not passing
  on this new tidy check.

TODO: this code has to be refactored to support more general cases, such as tab or newline.

- [X] `./mach build -d` does not report any errors
- [X] ./mach test-tidy does not report any errors
- [X] These changes fix #14898 (github issue number if applicable).
- [X] These changes do not require tests because ./mach test-tidy itself is the test for the code.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15690)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2017

💔 Test failed - linux-rel-css

@jdm
Copy link
Member

jdm commented Feb 23, 2017

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2017

Previous build results for android, arm32, arm64, linux-dev, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, windows-gnu-dev, windows-msvc-dev are reusable. Rebuilding only linux-rel-css, mac-rel-wpt2...

@bors-servo
Copy link
Contributor

bors-servo commented Feb 23, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
Approved by: Wafflespeanut
Pushing b34fdf6 to master...

@bors-servo bors-servo merged commit ccb14ab into servo:master Feb 23, 2017
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
@kimumetal kimumetal deleted the kimumetal:issue_14898 branch Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.