Skip to content

Conversation

@00Kai0
Copy link
Contributor

@00Kai0 00Kai0 commented Oct 20, 2019

No description provided.

Copy link
Member

@lresende lresende left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but leaving it open for a day or so to give a chance for the original reviewers to say something.
@minrk @takluyver

@lresende lresende added this to the 6.0.2 milestone Oct 21, 2019
@00Kai0
Copy link
Contributor Author

00Kai0 commented Oct 22, 2019

@lresende ,thank you for your approval.
I think I have removed most of py2 dependence.
And I find other problems that should be solved:

  • Some code imported but unused.
  • Replace gen.coroutine to async/await, because we just want to support >=py3.5.
  • Some code don't follow pep 8code style (if we want to follow this style).

@takluyver takluyver merged commit eb3e1a4 into jupyter:master Oct 29, 2019
@takluyver
Copy link
Member

Feel free to open PRs to clean up unused imports, and we can probably convert coroutines to async/await syntax. I'd say try to work piecewise where practical - small to medium PRs are easier to review than large ones.

Follow PEP 8 where you're adding or changing code anyway, but please don't reformat huge amounts of code that you're not otherwise touching - that causes merge conflicts unnecessarily and makes git blame less useful.

@takluyver
Copy link
Member

PS: When I say "follow PEP 8", I mean roughly. As you can probably tell, we're not strict about this - if leaving a line a couple of characters over the limit is more readable, for example, that's OK.

@00Kai0
Copy link
Contributor Author

00Kai0 commented Oct 30, 2019

@takluyver I agree with you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants