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

Warn if no visibility is specified on contract functions. #2749

Merged
merged 4 commits into from Sep 14, 2017

Conversation

Projects
None yet
2 participants
@axic
Member

axic commented Aug 15, 2017

Part of #2608. Depends on #2857.

@axic axic added the in progress label Aug 15, 2017

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Aug 21, 2017

Member

Need to change test cases.

Member

axic commented Aug 21, 2017

Need to change test cases.

@chriseth

This comment has been minimized.

Show comment
Hide comment
@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Sep 5, 2017

Member

How about the constructor? I think we had an issue it should only support public and as such it must also display the warning?

Member

axic commented Sep 5, 2017

How about the constructor? I think we had an issue it should only support public and as such it must also display the warning?

@axic axic added the nextrelease label Sep 13, 2017

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Sep 14, 2017

Member

We agreed that currently it does not make sense to enforce no-warnings on the end-to-end tests (and as such the specifiers should not be added above), because having "too pure" (as in up to latest standards) code would remove the possibility of getting old code tested.

Member

axic commented Sep 14, 2017

We agreed that currently it does not make sense to enforce no-warnings on the end-to-end tests (and as such the specifiers should not be added above), because having "too pure" (as in up to latest standards) code would remove the possibility of getting old code tested.

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Sep 14, 2017

Member

@chriseth please review

Member

axic commented Sep 14, 2017

@chriseth please review

@chriseth chriseth merged commit 934b0d2 into develop Sep 14, 2017

2 checks passed

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

@axic axic deleted the require-visibility branch Sep 14, 2017

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