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

Update error E0035 and E0036 #35206 #35207 #35561

Closed
wants to merge 1 commit into from
Closed

Update error E0035 and E0036 #35206 #35207 #35561

wants to merge 1 commit into from

Conversation

neilzxu
Copy link

@neilzxu neilzxu commented Aug 10, 2016

Part of #35233
Addresses #35207 #35206

I got

test result: ok. 2443 passed; 0 failed; 13 ignored; 0 measured

from running:

 python src/bootstrap/bootstrap.py --step check-cfail --stage 1

so I didn't update any tests.

r? @jonathandturner

@rust-highfive
Copy link
Collaborator

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@sophiajt
Copy link
Contributor

sophiajt commented Aug 10, 2016

Thanks for the PR!

There's a quirk in the test system where it doesn't check the labels if there aren't any in the test. I talk about it in the Extra Credit section of the blog post. Please update the tests so that it's testing the new labels. With that, we'll be ready.

@neilzxu
Copy link
Author

neilzxu commented Aug 11, 2016

@jonathandturner Alright, updated tests!

@sophiajt
Copy link
Contributor

Looks like it's almost there! Nice. A couple last comments:

  1. can we squash the 3 commits down to 1?
  2. I should have mentioned this in my issue, but we might want to slightly change the label wording. For example, if we pass 1 and it expects 2, it will say:
Passed 1 type arguments, expected 2

Can we make that:

Passed 1 type argument(s), expected 2

Or, alternatively, you could check the number of parameters yourself. Either way works with me.

With these fixed I think we'll be good.

@bors
Copy link
Contributor

bors commented Aug 17, 2016

☔ The latest upstream changes (presumably #35605) made this pull request unmergeable. Please resolve the merge conflicts.

Updated tests

fixes typo

Fix to account for plurals
@sophiajt
Copy link
Contributor

sophiajt commented Aug 24, 2016

Looks like you'll need to update to satisfy tidy.

/build/src/librustc_typeck/check/method/confirm.rs:321: line longer than 100 chars

thread 'main' panicked at 'some tidy checks failed', /build/src/tools/tidy/src/main.rs:53

You can run tidy with:

python src/bootstrap/bootstrap.py --stage 1 --step check-tidy

@bors
Copy link
Contributor

bors commented Oct 1, 2016

☔ The latest upstream changes (presumably #36885) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member

Fixed by another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants