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.

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

PR: Fix unblock comments #5017

Merged
merged 3 commits into from
Sep 8, 2017
Merged

Conversation

timhoffm
Copy link
Contributor

Fixes #4313.

@pep8speaks
Copy link

pep8speaks commented Aug 20, 2017

Hello @timhoffm! Thanks for updating the PR.

Line 887:1: W293 blank line contains whitespace
Line 890:80: E501 line too long (80 > 79 characters)
Line 891:50: E251 unexpected spaces around keyword / parameter equals
Line 891:52: E251 unexpected spaces around keyword / parameter equals
Line 891:80: E501 line too long (83 > 79 characters)
Line 899:1: W293 blank line contains whitespace
Line 903:66: W291 trailing whitespace

Comment last updated on August 29, 2017 at 18:44 Hours UTC

@timhoffm
Copy link
Contributor Author

The pep8 checker above moans about the line length. I would be reluctant to wrap in the above case.

Do you have a policy for the line length? I've noticed that most code sticks to ~80 chars, but there are some exceptions.

While we're at it, do you have a documented code style? I've not found anything on this.

@jnsebgosselin
Copy link
Member

@ccordoba12 I've tested it locally and this is working as expected. Maybe this should be rebased to the 3.x branch since this is a bug fix?

Maybe a test should also be added to test this feature?

@ccordoba12 ccordoba12 added this to the v3.2.3 milestone Aug 28, 2017
@ccordoba12
Copy link
Member

Maybe this should be rebased to the 3.x branch since this is a bug fix?

Yes, I think so too. @timhoffm, please follow these instructions to perform the rebase:

https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md#changing-base-branch

Also, please fix the style issues reported by Pep8speaks.

@ccordoba12 ccordoba12 changed the base branch from master to 3.x August 30, 2017 06:51
@timhoffm
Copy link
Contributor Author

@ccordoba12, I've fixed the pep8.

Any idea why AppVeyor sometimes hangs at the test spyder/plugins/tests/test_ipythonconsole.py::test_console_import_namespace? For the first commit 2.7 hung but 3.6 ran. Now, with no change but some extra line breaks 2.7 ran, but 3.6 hung. I suspect this is not related to my code.

@ccordoba12 ccordoba12 changed the title fix unblockcomment (#4313) PR: Fix unblock comment method Sep 7, 2017
@ccordoba12 ccordoba12 changed the title PR: Fix unblock comment method PR: Fix unblock comments Sep 7, 2017
@ccordoba12
Copy link
Member

@timhoffm, the failure was unrelated to your work. Thanks a lot for your contribution!

@ccordoba12 ccordoba12 merged commit 1e186e4 into spyder-ide:3.x Sep 8, 2017
ccordoba12 added a commit that referenced this pull request Sep 8, 2017
@timhoffm timhoffm deleted the unblockcomment branch September 9, 2017 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants