-
Notifications
You must be signed in to change notification settings - Fork 140
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
Add visit_IfExp and tests #207
Conversation
Pull Request Test Coverage Report for Build 742
💛 - Coveralls |
Thanks for the PR! @RJ722 could you review the code, please? |
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.
Just a minor nit. Otherwise, it's good to go from my side! Thanks @AgathiyanB!
tests/test_conditions.py
Outdated
foo if False else bar | ||
""" | ||
) | ||
check_unreachable(v, 1, 1, "ternary") |
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.
Let's end with a newline here.
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've amended the commit and pushed it with the newline a the end of the file now. Will that have amended this PR as well or should this be fine?
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.
Working, wonderful!
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.
The code looks great! I only have two minor comments.
CHANGELOG.md
Outdated
@@ -1,5 +1,6 @@ | |||
# Unreleased | |||
|
|||
* Detect unreachable code in conditional expressions (#178) |
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.
If you like, please add your name or username inside the brackets and end the sentence with a dot.
tests/test_conditions.py
Outdated
v.scan( | ||
"""\ | ||
foo if True else bar | ||
""" |
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.
For both new tests, you can use a normal string on a single line (so that each function body uses two lines).
Sorry, there was a merge conflict in CHANGLELOG.md, I think it should be resolved now. |
Thanks! |
Add 'visit_IfExp' method
Description
Added 'visit_IfExp' method and changed the '_handle_conditional_node' method adding cases for the new method. Tests for the new method have also been added.
Related Issue
#178
Checklist: