Skip to content
This repository was archived by the owner on Oct 8, 2023. It is now read-only.

[feat] PEP8 Support#110

Open
Zheaoli wants to merge 8 commits into
vibora-io:masterfrom
Zheaoli:master
Open

[feat] PEP8 Support#110
Zheaoli wants to merge 8 commits into
vibora-io:masterfrom
Zheaoli:master

Conversation

@Zheaoli

@Zheaoli Zheaoli commented Jul 2, 2018

Copy link
Copy Markdown
  • add flake8 checking in CI

  • add flake8 config file to ignore some unnecessary option such as:E501 line too long ,F401 '.router.*' imported but unused

  • fix some code style problem

Comment thread .circleci/config.yml Outdated
name: Code Style Check
command: |
. venv/bin/activate
flake8 vibora

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This misses the Python syntax error in #32 that flake8 is capable of finding.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

all right,I will recovery it

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@cclauss
I check this issue. The author has fixed this problem.

@Zheaoli

Zheaoli commented Jul 17, 2018

Copy link
Copy Markdown
Author

@frnkvieira

@cclauss

cclauss commented Jul 17, 2018

Copy link
Copy Markdown
Contributor

Why flake8 vibora and not flake8 . to test the entire repo? Don’t we want to also automate the process of finding issues in our ./samples and ./tests?

@Zheaoli

Zheaoli commented Jul 17, 2018

Copy link
Copy Markdown
Author

@cclauss OK I will fix it

@cclauss cclauss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work!!

@Zheaoli

Zheaoli commented Jul 18, 2018

Copy link
Copy Markdown
Author

@frnkvieira

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants