Skip to content

Check that CI enforces rustfmt #121

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
Merged
merged 2 commits into from
Aug 27, 2019
Merged

Check that CI enforces rustfmt #121

merged 2 commits into from
Aug 27, 2019

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Jun 19, 2019

Sanity check that CI actually enforces rustfmt. I suspect something is broken.

@coveralls
Copy link

coveralls commented Jun 19, 2019 •
edited
Loading

Pull Request Test Coverage Report for Build 367

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%
Totals Coverage Status
Change from base Build 358: 0.0%
Covered Lines:
Relevant Lines: 0
💛 - Coveralls

@benesch
Copy link
Contributor Author

benesch commented Jun 19, 2019

Well, that proves it.

@benesch
Copy link
Contributor Author

benesch commented Jun 25, 2019

Ok, turns out the crash is a rustfmt bug that I've filed upstream: rust-lang/rustfmt#3655.

@benesch
Copy link
Contributor Author

benesch commented Jun 25, 2019

Ok, once rustfmt is fixed we can squash this PR down and merge, and we'll actually have rustfmt working in CI. Whoops!

benesch added 2 commits August 13, 2019 15:03
@benesch
Actually check rustfmt in CI
bc97543 
The previous incantation was simply *never* running rustfmt in CI,
rather than only running rustfmt on the nightly build.
@benesch
Add missing license header
070d019
@benesch benesch changed the title [do-not-merge] check that ci enforces rustfmt Check that ci enforces rustfmt Aug 13, 2019
@benesch benesch requested a review from nickolay August 13, 2019 19:05
@benesch
Copy link
Contributor Author

benesch commented Aug 13, 2019

@nickolay, does this LGTY? The panic in rustfmt (rust-lang/rustfmt#3655) is finally fixed in nightly, so should be good to go.

@benesch benesch changed the title Check that ci enforces rustfmt Check that CI enforces rustfmt Aug 14, 2019
nickolay
nickolay approved these changes Aug 27, 2019
View reviewed changes
Copy link
Contributor

@nickolay nickolay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I won't claim to understand how this works, so thanks a lot for figuring this out!

@benesch
Copy link
Contributor Author

benesch commented Aug 27, 2019

Yeah I’m not sure how much value we’re getting out of travis-cargo, since it obscures what’s going on just to provide coverage information. But at least it works now! Thanks for the review.

@benesch benesch merged commit 2bef9ec into master Aug 27, 2019
@benesch benesch deleted the ci-fmt branch August 27, 2019 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickolay nickolay nickolay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@benesch @coveralls @nickolay