-
Notifications
You must be signed in to change notification settings - Fork 6.3k
updated public to external for the functions #11772
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
updated public to external for the functions #11772
Conversation
| function winnerName() external view | ||
| returns (bytes32 winnerName_) | ||
| { | ||
| winnerName_ = proposals[winningProposal()].name; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
winningProposal is used here. So that has to be made public. Here's the CI check that failed: https://app.circleci.com/pipelines/github/ethereum/solidity/17880/workflows/1a6d2776-037e-462b-a52c-a88feed5f637/jobs/799349
Could you please fix it in this PR / branch itself? We have a section on workflow for pull requests that may be relevant here: https://docs.soliditylang.org/en/v0.8.6/contributing.html?highlight=evmone#workflow-for-pull-requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok i will use the same PR request to make new changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. You can ping me by using "re request" review button next to the Reviewers list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also update other files from docs/examples/*.rst?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course i will update the others as well
|
Generally looks good. I'll take another look after the rest of the examples are changed. It would be nice to squash the commits into a single one before we can merge it. But I can also do it at the end, if you aren't familiar with this. |
|
Please make sure that the explanation before and after the code is still OK with the code. |
|
Will squash the commits now. |
changed public to external for the functions
e99aaf7 to
9b9e52e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
|
@v-sreekesh Thanks for the PR! The CI seems to be broken now. Will merge once it gets fixed. |
|
It's only the windows build that is failing. Will force-merge this. |
changed public to external for the functions
Closes #11768