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.

Sign up for GitHub

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

Jump to bottom

Feature: Linter (flake8) on GitHub Actions #102

Merged
merged 2 commits into from
Aug 25, 2020
Merged

Feature: Linter (flake8) on GitHub Actions #102

merged 2 commits into from
Aug 25, 2020

Conversation

ymd-h
Copy link
Contributor

@ymd-h ymd-h commented Aug 11, 2020

For #101

This PR adds a job on GitHub Actions to run flake8 (Linter) at push and pull request.

This utilizes GitHub Super-Linter.

According to the previous PR (#95), some warnings and errors are ignored, which can be configurable at .github/linters/.flake8.

  • E129: visually indented line with same indent as next logical line
  • E226: missing whitespace around arithmetic operator
  • E303: too many blank lines (X)
  • E501: line too long (XX > 79 characters)
  • W503: line break before binary operator
  • W504: line break after binary operator
  • E741: do not use variables named ‘l’, ‘O’, or ‘I’

Additionally, a 3rd party copied file tf2rl/misc/prepare_output_dir.py is ignored, too.

Some code style is also fixed. (58fb730)

ymd-h added 2 commits August 11, 2020 08:25
@ymd-h
Add super linter configuration
2e58db3
@ymd-h
Fix Flake8 Errors
58fb730 
E231: Add space after comma (,)
E302: Add blank line
E305: Add blank line
F401: Remove unused import module
E704: Make multi-line
F821: Undefined standard_ops is replaced by tf
F841: Remove unused local variables
@keiohta
Copy link
Owner

keiohta commented Aug 25, 2020

Thanks for the really useful PR!

I would like to confirm the following two things:

  1. Does this PR depend on your previous PR (Reduce tf.function retracing #100 )?
  2. Does this PR force to edit codes so that they satisfy flake8, or just show messages (warnings)?

@ymd-h
Copy link
Contributor Author

ymd-h commented Aug 25, 2020

@keiohta

Does this PR depend on your previous PR (#100 )?

No. These are independent.

Does this PR force to edit codes so that they satisfy flake8, or just show messages (warnings)?

No. This PR shows linter "test" results for all PRs and pushes.
As you can see, this PR also has "red" results of coverage/coveralls (because of decrease of coverage), however, you can still merge this.
The decision still depends on you.

@keiohta keiohta merged commit 5e188a0 into keiohta:master Aug 25, 2020
@keiohta
Copy link
Owner

keiohta commented Aug 25, 2020

@ymd-h Thanks for quick reply. I merged this.

@ymd-h ymd-h deleted the Feature_linter branch August 27, 2020 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ymd-h @keiohta