Upgrade jerry-debugger client to use python3#4825
Conversation
galpeter
left a comment
galpeter
left a comment
There was a problem hiding this comment.
General question: do we want to remove full python2 support?
| self.quit = True | ||
| return | ||
| elif res_type == result.TEXT: | ||
| if res_type == result.TEXT: |
There was a problem hiding this comment.
Why do we need to change elif to if? This applies to most of the cases.
There was a problem hiding this comment.
Pylint does not accepts elif here becasue of return in previous if statement:
************* Module jerry_client
jerry-debugger/jerry_client.py:104:20: R1705: Unnecessary "elif" after "return" (no-else-return)
|
|
||
| protocol = Socket(address) | ||
| elif args.protocol == "serial": | ||
| from jerry_client_serial import Serial |
There was a problem hiding this comment.
Do we really need to move this out of here? If so, we always expect that a user will install the serial package even if they will only use the socket based communication.
There was a problem hiding this comment.
Moved import to top level because of pylint.
************* Module jerry_client
jerry-debugger/jerry_client.py:281:8: C0415: Import outside toplevel (jerry_client_serial) (import-outside-toplevel)
There was a problem hiding this comment.
I can move back the import there and disable pylint on that row.
There was a problem hiding this comment.
This was only error locally so I reverted this.
| breakpoint = JerryBreakpoint(_line, offset, self) | ||
| self.lines[_line] = breakpoint | ||
| self.offsets[offset] = breakpoint | ||
| br_point = JerryBreakpoint(_line, offset, self) |
There was a problem hiding this comment.
Why do we need to rename the "breakpoint" variable? This also applies to other places.
There was a problem hiding this comment.
Rename is needed because breakopoint is a built-in.
************* Module jerry_client_main
jerry-debugger/jerry_client_main.py:215:12: W0622: Redefining built-in 'breakpoint' (redefined-builtin)
| return "Error: Breakpoint index expected\n" \ | ||
| "Delete the given breakpoint, use 'delete all|active|pending' " \ | ||
| "to clear all the given breakpoints\n " | ||
| elif args in ['all', 'pending', 'active']: |
There was a problem hiding this comment.
Why change the "elif" to "if" ?
| return "" | ||
|
|
||
| elif buffer_type in [JERRY_DEBUGGER_SOURCE_CODE, JERRY_DEBUGGER_SOURCE_CODE_END]: | ||
| if buffer_type in [JERRY_DEBUGGER_SOURCE_CODE, JERRY_DEBUGGER_SOURCE_CODE_END]: |
| for function in list(new_function_list.values()): | ||
| for line, br_point in list(function.lines.items()): |
There was a problem hiding this comment.
Why do we need the list calls?
| for src in list(new_function_list.values()): | ||
| if (src.source_name == br_point.source_name or | ||
| src.source_name.endswith("/" + br_point.source_name) or | ||
| src.source_name.endswith("\\" + br_point.source_name)): | ||
| source_lines = len(src.source) | ||
| break |
There was a problem hiding this comment.
Why do we need the list call here?
ad99e49 to
69c41a5
Compare
|
Updated the PR. 2to3 app changed the parenthesis, next function and list call. |
Also upgrade pylint to use python3. JerryScript-DCO-1.0-Signed-off-by: Csaba Repasi repasics@inf.u-szeged.hu
69c41a5 to
809a83e
Compare
|
this can be closed, python3 is already supported? |
|
I've just closed this PR due to python3 is already supported. |
7 participants
Also upgrade pylint to use python3.
JerryScript-DCO-1.0-Signed-off-by: Csaba Repasi repasics@inf.u-szeged.hu