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

Use level "note" for reveal_type and reveal_locals #6919

Merged
merged 4 commits into from Jun 18, 2019

Conversation

@blueyed
Copy link
Contributor

commented Jun 1, 2019

Fixes #6901.

TODO:

  • indentation with reveal_locals
@ilevkivskyi
Copy link
Collaborator

left a comment

In principle I like this change (you can also indent revealed locals as discussed in the issue). But this is changes huge amount of tests, so I think this requires approval from most of core team members.

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2019

Great.

so I think this requires approval from most of core team members.

Ok, no hurry - I can see that this might cause trouble with pending PRs etc.

@ethanhs

ethanhs approved these changes Jun 3, 2019

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

  • indentation with reveal_locals

As for this: do you think prepending " - " is good here?

@JukkaL

JukkaL approved these changes Jun 3, 2019

Copy link
Collaborator

left a comment

I like this change. It may be worth coordinating when to merge this as this goes stale quickly.

@blueyed blueyed force-pushed the blueyed:reveal-note branch from 87b163e to 6192e39 Jun 3, 2019

@blueyed blueyed changed the title [RFC/WIP] Use level "note" for reveal_type and reveal_locals Use level "note" for reveal_type and reveal_locals Jun 3, 2019

@ilevkivskyi

This comment has been minimized.

Copy link
Collaborator

commented Jun 3, 2019

As for this: do you think prepending " - " is good here?

I think just four spaces offset is enough (like in protocol error message notes)

@blueyed blueyed force-pushed the blueyed:reveal-note branch from 6192e39 to 85f5df0 Jun 5, 2019

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Jun 5, 2019

Ok, updated and rebased.

@msullivan

This comment has been minimized.

Copy link
Collaborator

commented Jun 17, 2019

Sure! If you rebase it again I'll merge it.

blueyed added some commits Jun 1, 2019

Update tests
- sed -i 's/# E: Revealed type/# N: Revealed type/g' test-data/**/*.test
- sed -i 's/: error: Revealed type/: note: Revealed type/g' test-data/**/*.test

@blueyed blueyed force-pushed the blueyed:reveal-note branch from 85f5df0 to 8270705 Jun 18, 2019

@blueyed

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2019

@msullivan
Ok, hope it stays green.

@msullivan msullivan merged commit cb7b242 into python:master Jun 18, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@blueyed blueyed deleted the blueyed:reveal-note branch Jun 18, 2019

gvanrossum added a commit that referenced this pull request Jun 18, 2019

Use level "note" for reveal_type and reveal_locals (#6919)
Tests were fixed using:
- sed -i 's/# E: Revealed type/# N: Revealed type/g' test-data/**/*.test
- sed -i 's/: error: Revealed type/: note: Revealed type/g' test-data/**/*.test

Fixes #6901.
@gvanrossum gvanrossum referenced this pull request Jun 18, 2019

PattenR added a commit to PattenR/mypy that referenced this pull request Jun 23, 2019

Use level "note" for reveal_type and reveal_locals (python#6919)
Tests were fixed using:
- sed -i 's/# E: Revealed type/# N: Revealed type/g' test-data/**/*.test
- sed -i 's/: error: Revealed type/: note: Revealed type/g' test-data/**/*.test

Fixes python#6901.

ikonst added a commit to pynamodb/PynamoDB that referenced this pull request Jun 25, 2019

tests: bump to mypy 0.710
Update mypy output parser for python/mypy#6919
@ikonst ikonst referenced this pull request Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.