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

Fix ICE when '__init__' returns any other type than '()' #323

Merged
merged 2 commits into from
Mar 22, 2021

Conversation

Y-Nak
Copy link
Member

@Y-Nak Y-Nak commented Mar 22, 2021

fixes #307.

What was wrong?

Semantic checks missed __init__ return type.

How was it fixed?

Assert the return type is ().

  • OPTIONAL: Update Spec if applicable
  • Add entry to the release notes (may forgo for trivial changes)
  • Clean up commit history

Copy link
Collaborator

@cburgdorf cburgdorf left a comment

Choose a reason for hiding this comment

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

Olá Yoshitomo,

thanks for the fix. Looks good 👍
Can you also add a file 323.bugfix.md to the newsfragments directory with the content being something like:

Ensure analyzer rejects code that uses return values for __init__ function

This is so that the bugfix will be mentioned in the upcoming release notes.

@Y-Nak
Copy link
Member Author

Y-Nak commented Mar 22, 2021

Changes:

  • Add a file to newsfragments
  • Replace ReST with md in newsfragments/README.md

Question:
the README.md says

If the PR fixes an issue, use that number here. If there is no issue, then open up the PR first and use the PR number for the
newsfragment.

while it seems PR number is used currently even if the corresponding issue is opened.
Should I also fix this document?

@cburgdorf
Copy link
Collaborator

If the PR fixes an issue, use that number here. If there is no issue, then open up the PR first and use the PR number for the
newsfragment.

I think that advice is still correct even though we often use the PR number even though the issue number would be slightly preferable. It's just a minor sloppiness on our side 🙃

@Y-Nak
Copy link
Member Author

Y-Nak commented Mar 22, 2021

Thanks for clarifying! I leave it as is then.

@cburgdorf cburgdorf merged commit fb958cd into ethereum:master Mar 22, 2021
@cburgdorf
Copy link
Collaborator

Landed 🎉 Btw, I'm taking care of the failed coverage job in #325
It is unrelated to your changes.

@Y-Nak Y-Nak deleted the fix-ice-307 branch March 22, 2021 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants