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

rustfmt tests #3529

Merged
merged 8 commits into from
Dec 12, 2018
Merged

rustfmt tests #3529

merged 8 commits into from
Dec 12, 2018

Conversation

matthiaskrgr
Copy link
Member

also fails CI if tests turn out to not be formatted properly.

phansch
phansch suggested changes Dec 11, 2018
View reviewed changes
Copy link
Member

@phansch phansch left a comment

Choose a reason for hiding this comment

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

Apart from what @sinkuu said, it would be good to include the subdirectories, too. Otherwise this LGTM.

ci/base-tests.sh Show resolved Hide resolved
tests/ui/decimal_literal_representation.rs Outdated Show resolved Hide resolved
tests/ui/decimal_literal_representation.rs Outdated Show resolved Hide resolved
tests/ui/implicit_hasher.rs Outdated Show resolved Hide resolved
tests/ui/swap.rs Outdated Show resolved Hide resolved
@phansch phansch added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Dec 11, 2018
@matthiaskrgr matthiaskrgr added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 11, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Dec 11, 2018

appveyor is unhappy

@phansch
Copy link
Member

phansch commented Dec 11, 2018

@oli-obk yup, that's probably going to be fixed by rust-lang-deprecated/failure#279

@matthiaskrgr
Copy link
Member Author

@bors try

@bors
Copy link
Contributor

bors commented Dec 11, 2018

⌛ Trying commit 36266b3 with merge 301d44a...

bors added a commit that referenced this pull request Dec 11, 2018
@bors
Auto merge of #3529 - matthiaskrgr:rustfmt_tests, r=<try>
301d44a 
rustfmt tests

also fails CI if tests turn out to not be formatted properly.
@matthiaskrgr
Copy link
Member Author

./ci/base-tests.sh: line 44: [: missing ]
I should check that out

@bors
Copy link
Contributor

bors commented Dec 12, 2018

☀️ Test successful - status-appveyor, status-travis
State: approved= try=True

@phansch
Copy link
Member

phansch commented Dec 12, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Dec 12, 2018

📌 Commit 36266b3 has been approved by phansch

@bors
Copy link
Contributor

bors commented Dec 12, 2018

⌛ Testing commit 36266b3 with merge e2608fc...

bors added a commit that referenced this pull request Dec 12, 2018
@bors
Auto merge of #3529 - matthiaskrgr:rustfmt_tests, r=phansch
e2608fc 
rustfmt tests

also fails CI if tests turn out to not be formatted properly.
@bors
Copy link
Contributor

bors commented Dec 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: phansch
Pushing e2608fc to master...

@bors bors merged commit 36266b3 into rust-lang:master Dec 12, 2018
This was referenced Dec 12, 2018
Closed
Merged
Closed
This was referenced Dec 12, 2018
Merged
Merged
@phansch phansch mentioned this pull request Dec 12, 2018
Merged
@matthiaskrgr
Copy link
Member Author

The fact that CI was green while there was a syntax error in the script is actually quite concerning...
I'll look into this.

@mati865
Copy link
Contributor

mati865 commented Dec 12, 2018

I'd didn't exit because it was told not to exit on error

rust-clippy/ci/base-tests.sh

Line 35 in 921d4da

set +ex

@matthiaskrgr matthiaskrgr mentioned this pull request Dec 12, 2018
Merged
bors added a commit that referenced this pull request Dec 12, 2018
@bors
Auto merge of #3537 - matthiaskrgr:fix_script_2, r=matthiaskrgr
f5d6aca 
base-tests: don't print all commands to stdout during the loop

Fixes thinko in #3529
@matthiaskrgr
Copy link
Member Author

Fixed via #3537

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sinkuu sinkuu sinkuu left review comments

@phansch phansch phansch approved these changes

Assignees
No one assigned
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@matthiaskrgr @oli-obk @phansch @bors @mati865 @sinkuu