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

Add flake8 extensions to linter CI workflow #560

Merged
merged 5 commits into from
Nov 15, 2022

Conversation

itrushkin
Copy link
Contributor

@itrushkin itrushkin commented Nov 2, 2022

This pull request contains:

  • Including installation of flake8 extensions
  • Making linter workflow fail on any non-ignored (see setup.cfg) error.
  • Removing linter rules specified directly in workflow code. All rules have already been specified in setup.cfg file in the repository root.
  • Fixing all linter errors

Tested the behavior locally via act.

@itrushkin
Copy link
Contributor Author

New flake8 rules have been applied. I will convert this to a draft and fix all the issues here.

@itrushkin itrushkin marked this pull request as draft November 2, 2022 09:29
@itrushkin itrushkin marked this pull request as ready for review November 3, 2022 16:41
Copy link
Contributor

@igor-davidyuk igor-davidyuk left a comment

Choose a reason for hiding this comment

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

Great, thank you!
One thing that I would do is separate this into two PRs.
One will contain changes to CI and another one with linter highlited fixes.

@itrushkin
Copy link
Contributor Author

@igor-davidyuk It seems that CI changes affect GitHub actions immediately. I had linter errors after enabling the extensions, thus I had to fix them in the same pull request.

@igor-davidyuk
Copy link
Contributor

@igor-davidyuk It seems that CI changes affect GitHub actions immediately. I had linter errors after enabling the extensions, thus I had to fix them in the same pull request.

Yeah, I thought it was the case. We could Merge a PR with fixes first to avoid this problem 🤣
But this is not that important, one combined PR is ok too.

Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>
Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>
Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>
@psfoley psfoley merged commit 66623b8 into develop Nov 15, 2022
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