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

Drop pylint check? #980

Closed
yongtang opened this issue May 21, 2020 · 3 comments
Closed

Drop pylint check? #980

yongtang opened this issue May 21, 2020 · 3 comments

Comments

@yongtang
Copy link
Member

yongtang commented May 21, 2020

There were some discussions about dropping pylint before #810 (comment)

Several big issues for pylint:

  1. Slow: pylint runs check 1-2 second/file while black and pyupgrade runs very fast.
  2. No auto-lint: It has to be adjusted manually. In some situations, you have to be really creative to make pylint happy.

Positive side:

  • tensorflow core repo stays with pylint so it helps us to align with the community. Though we change the max-width to 88 (not 80), and use 4 spaces (not 2) already so our pylint script is not exactly from tensorflow's core repo already.

On a side node, to submit a PR to tensorflow core repo and make pylint happy is not very easy nowadays. This actually caused quite a few unnecessary commits in tensorflow' core repo.

We might consider dropping pylint if agree? @terrytangyuan @BryanCutler @vlasenkoalexey @dmitrievanthony

@terrytangyuan
Copy link
Member

Yes agree that we can drop it.

@BryanCutler
Copy link
Member

SGTM

@yongtang
Copy link
Member Author

The issue can be closed now as PR #985 has been merged.

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

No branches or pull requests

3 participants