-
-
Notifications
You must be signed in to change notification settings - Fork 819
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
feat[ux]: improve hint for events kwarg upgrade #4275
base: master
Are you sure you want to change the base?
feat[ux]: improve hint for events kwarg upgrade #4275
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4275 +/- ##
=======================================
Coverage 92.09% 92.09%
=======================================
Files 119 119
Lines 16931 16933 +2
Branches 2865 2865
=======================================
+ Hits 15593 15595 +2
Misses 919 919
Partials 419 419 ☔ View full report in Codecov by Sentry. |
there's something interesting going on with examples like
i guess mark_tokens() marks the wrong start+end position for the ast node. |
i'd say the mark_tokens issue is out of scope -- it should be addressed in #4364 |
let's please add some tests |
it seems that not all nodes have the source assigned? event Foo:
a: uint256
@external
def foo(a: uint256):
log Foo(a)
Foo(a=)
notice |
Yea that's what I mentioned here |
oh, that wasn't clear to me from the comment, but i didn't run the attached code in that case can we add 1 xfail test please? |
…ove-event-kwargs-recommendation add tests for event kwargs hint
note: waiting on #4364. we should update the logs in the examples directory in this PR as well. |
recommendation = f"log {node.func.node_source_code}({rec0})" | ||
msg = "Instantiating events with positional arguments is" | ||
msg += " deprecated as of v0.4.1 and will be disallowed" | ||
msg += " in a future release. Use kwargs instead e.g.:" |
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.
do we need e.g. when we have a concrete recommendation?
follow-on commit to ebe3c0c. the hint in ebe3c0c is not specific to the user's source code, and it's a bit laborious for the user to match the call-site to the event def. this commit auto-generates the new call expression for the user to use.
What I did
How I did it
How to verify it
try compiling the following file:
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture