- Notifications
You must be signed in to change notification settings - Fork1.4k
Move set_item to DictProtocol#548
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
vm/src/macros.rs Outdated
| #[allow(unused_assignments)] | ||
| { | ||
| arg_count +=1; | ||
| let len = $args.args.len(); |
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.
Wait, this change looks really weird.
Builtin functions can have optional arguments that are not keyword arguments. They way you did it, you seem to assume that optional arguments canonly be kwargs. This breaks it completely. As it was,arg_check! simply handled positional arguments.
(that said, admittably handling optional arguments that can be either positional or keyword is a bit awkward in the current model)
Also, it seems to assume that kwargs are passed in the same order as they were declared?
(Also, shouldn't this change be in a separate PR?)
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.
I reverted the changes
codecov-io commentedFeb 27, 2019
Codecov Report
@@ Coverage Diff @@## master #548 +/- ##==========================================- Coverage 43.06% 42.25% -0.82%========================================== Files 73 73 Lines 15924 16144 +220 Branches 4179 4239 +60 ==========================================- Hits 6858 6821 -37- Misses 7144 7401 +257 Partials 1922 1922
Continue to review full report at Codecov.
|
Uh oh!
There was an error while loading.Please reload this page.
This commits moves set_item method from PyContext to DictProtocol trait.