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

Remove pyflakes from CI #4371

Merged
merged 1 commit into from Jul 22, 2022
Merged

Remove pyflakes from CI #4371

merged 1 commit into from Jul 22, 2022

Conversation

justinchuby
Copy link
Contributor

Description

pyflakes does not support diff_context, creating too much noise. So we disable it.

  • Set diff_context in all other checks
  • We do not need to reenable pyflakes because flake8 is supposed to include pyflakes. We just need to configure properly

Signed-off-by: Justin Chu <justinchu@microsoft.com>
@justinchuby justinchuby requested a review from a team as a code owner July 22, 2022 17:14
@justinchuby
Copy link
Contributor Author

@jcwchen

Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for the updates! In the future we can add pyflakes back after it has supported diff_context.

@justinchuby
Copy link
Contributor Author

We prob don't need to reenable pyflakes because flake8 is supposed to include pyflakes. We just need to configure flake8 properly (to enable those checks)

@jcwchen
Copy link
Member

jcwchen commented Jul 22, 2022

We prob don't need to reenable pyflakes because flake8 is supposed to include pyflakes. We just need to configure flake8 properly (to enable those checks)

That's good to know. I thought pyflakes involves other linter as well (https://github.com/reviewdog/action-pyflakes#lint---reviewdog-integration), but actually ONNX Python Lint has covered some of them already: action-shellcheck and action-misspell. Then yes we don't need to reenable it. Updated my old comment.

@justinchuby
Copy link
Contributor Author

justinchuby commented Jul 22, 2022

I think that's just a templated readme that's not updated

@jcwchen
Copy link
Member

jcwchen commented Jul 22, 2022

I think that's just a templated readme that's not updated

You are right 👍

@jcwchen jcwchen merged commit 2f3d7fa into onnx:main Jul 22, 2022
broune pushed a commit to broune/onnx that referenced this pull request May 6, 2023
Signed-off-by: Justin Chu <justinchu@microsoft.com>
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.

None yet

2 participants