[gyb] check isinstance(result, basestring) before string comparison #7324
Conversation
|
@swift-ci please python lint. |
|
@swift-ci Please smoke test |
|
PyLint failures unrelated to this change. |
|
I'll open a PR fixing the python-lint issues by the way, just claiming that :D |
|
@toffaletti Looks like you don't have your collaborator bit, so CI has to be done by somebody else @swift-ci please smoke test |
| @@ -716,7 +716,8 @@ def execute(self, context): | |||
|
|
|||
| # If we got a result, the code was an expression, so append | |||
| # its value | |||
| if result is not None and result != '': | |||
| if result is not None \ | |||
| or (isinstance(result, basestring) and result != ''): | |||
There was a problem hiding this comment.
This won't work in Python 3. I think we try to make all the python code compatible with both.
There was a problem hiding this comment.
I'm pretty sure gyb would require a lot of changes to work with python3, but do you have a preferred solution?
There was a problem hiding this comment.
Out of curiousity I just ran gyb.py --verbose-test with both python and python3. Both succeeded. Which is a bit surprising since doctests contain a lot of print x statements. Anyhow, this looks alright.
There was a problem hiding this comment.
Done. I'm surprised too, pleasantly. The utils/gyb file explicitly referencing python2.7 and discussion in #1535 threw me off. I guess I also expected to see unicode_literals imported if the code was trying to be Python 3 ready, and didn't really pay attention to the rest of the code.
Guard against types that override __eq__ and do things like raise TypeError when the types being compared don't match.
|
@swift-ci Please smoke test and merge |
Guard against types that override
__eq__and do things likeraise TypeErrorwhen the types being compared don't match.