Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pyterm: enable TCP connection to remote host #1950

Merged
merged 2 commits into from
Nov 7, 2014
Merged

pyterm: enable TCP connection to remote host #1950

merged 2 commits into from
Nov 7, 2014

Conversation

OlegHahm
Copy link
Member

@OlegHahm OlegHahm commented Nov 5, 2014

No description provided.

@OlegHahm OlegHahm added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Nov 5, 2014
@phiros
Copy link
Member

phiros commented Nov 6, 2014

Tested it. Works like a charm.
However, I noticed that pep8 has quite a lot of complaints (~ 50) about pyterm in general.
This could be easily fixed by using autopep8 on pyterm.
Do you have time to do this?
Otherwise I could open an extra PR for this.

@OlegHahm
Copy link
Member Author

OlegHahm commented Nov 6, 2014

If the warnings are not introduced by this PR, I would love to merge this now and open an PR for the autopep thing.

@OlegHahm
Copy link
Member Author

OlegHahm commented Nov 6, 2014

Ok, fixed most of the pep8 complaints. The remaining ones would make things worse in my eyes.

@OlegHahm
Copy link
Member Author

OlegHahm commented Nov 7, 2014

Ok, I interpret your previous comment as an ACK and since the second commit is just a cleanup, I'll merge this now. (It's not a part of the OS anyway.)

OlegHahm added a commit that referenced this pull request Nov 7, 2014
@OlegHahm
Merge pull request #1950 from OlegHahm/pyterm_tcp_remote
1b64725 
pyterm: enable TCP connection to remote host
@OlegHahm OlegHahm merged commit 1b64725 into RIOT-OS:master Nov 7, 2014
@OlegHahm OlegHahm deleted the pyterm_tcp_remote branch November 7, 2014 11:21
@phiros
Copy link
Member

phiros commented Nov 7, 2014

Yep, that was meant as an ACK.

BTW: We don't have to fix every single pep8 issue. Some of the proposed fixes for issues such as E401 and E221 (in a certain contexts) would make matters worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@phiros phiros

Labels
Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@OlegHahm @phiros