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.

Sign up for GitHub

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

Jump to bottom

Update clippy, improve clippy integration #12409

Merged
merged 3 commits into from
Jul 12, 2016
Merged

Update clippy, improve clippy integration #12409

merged 3 commits into from
Jul 12, 2016

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Jul 12, 2016 •
edited by larsbergstrom
Loading

This makes it much easier to run rustfix on servo

(rustfix is still pretty buggy though)

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/Cargo.toml, components/style/lib.rs, components/style/custom_properties.rs, components/style/attr.rs, components/style/matching.rs
  • @wafflespeanut: python/servo/devenv_commands.py

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 12, 2016
@highfive
Copy link

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

@Manishearth Manishearth force-pushed the clippy branch 2 times, most recently from c698fa5 to 514180f Compare July 12, 2016 11:50
@nox
Copy link
Contributor

nox commented Jul 12, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 514180f has been approved by nox

@highfive highfive assigned nox and unassigned KiChjang Jul 12, 2016
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 12, 2016
@nox
Copy link
Contributor

nox commented Jul 12, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 514180f has been approved by nox

@jdm
Copy link
Member

jdm commented Jul 12, 2016

@bors-servo: r-
Travis is displeased.

@jdm jdm added S-fails-travis and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 12, 2016
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 12, 2016
@Manishearth
Copy link
Member Author

@bors-servo r=nox

fixed tidy

@bors-servo
Copy link
Contributor

📌 Commit 6e65607 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 12, 2016
@Manishearth
Copy link
Member Author

Sadly rustfix isn't too ready yet (a couple of crashes, and it messes up indentation), and clippy needs to improve its suggestions, so I wasn't able to do a proper fixing sweep. I'll try rustfix on some of our smaller crates later and go on from there.

@bors-servo
Copy link
Contributor

⌛ Testing commit 6e65607 with merge d257d15...

bors-servo pushed a commit that referenced this pull request Jul 12, 2016
Auto merge of #12409 - Manishearth:clippy, r=nox
d257d15 
Update clippy, improve clippy integration

This makes it much easier to run rustfix on servo

(rustfix is still pretty buggy though)

<!-- 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/12409)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - linux-dev

@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 12, 2016
@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Jul 12, 2016
@wafflespeanut
Copy link
Contributor

compile tests seem to have failed.

@wafflespeanut wafflespeanut reopened this Jul 12, 2016
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 12, 2016
@Manishearth
Copy link
Member Author

@bors-servo r=nox

@bors-servo
Copy link
Contributor

📌 Commit 2091981 has been approved by nox

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 12, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit 2091981 with merge 7492546...

bors-servo pushed a commit that referenced this pull request Jul 12, 2016
Auto merge of #12409 - Manishearth:clippy, r=nox
7492546 
Update clippy, improve clippy integration

This makes it much easier to run rustfix on servo

(rustfix is still pretty buggy though)

<!-- 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/12409)
<!-- Reviewable:end -->
@Manishearth
Copy link
Member Author

@bors-servo r=nox

I undid the change that only runs the plugins on script. Plugins take a fraction of a second for us, so it's no big deal.

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

  • This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

@bors-servo
Copy link
Contributor

📌 Commit 2091981 has been approved by nox

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 12, 2016
@highfive
Copy link 

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform3d-translatez-001.htm
  └   → /css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-ref.htm b14318ccfd8a59b7b249d41521e178d617678823
Testing 902d90a8d198625335e738</span><span class="stdout">587fdfe81dcc90392d == b14318ccfd8a59b7b249d41521e178d617678823
/css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-notref.htm 902d90a8d198625335e738587fdfe81dcc90392d
Testing 902d90a8d198625335e738587fdfe81dcc90392d != 902d90a8d198625335e738587fdfe81dcc90392d

@Manishearth
Copy link
Member Author

@bors-servo retry

@bors-servo
Copy link
Contributor

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

@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo merged commit 2091981 into servo:master Jul 12, 2016
@Manishearth Manishearth deleted the clippy branch May 7, 2019 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nox nox

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Manishearth @highfive @nox @bors-servo @jdm @wafflespeanut @KiChjang