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 config file #61836

Closed
wants to merge 1 commit into from

Conversation

@danielmellado
Copy link
Contributor

commented Sep 5, 2019

SUMMARY

Add flake8 config file

Even if ansible-test is handling pep8 testing, this commit brings back
and updates the missing flake8 config that went away when removing
legaxy tox.ini.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

flake8

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

The test ansible-test sanity --test package-data [explain] failed with 1 error:

.flake8:0:0: File was not added to sdist

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Sep 5, 2019

@@ -0,0 +1,4 @@
[flake8]

This comment has been minimized.

Copy link
@gundalow

gundalow Sep 5, 2019

Contributor

I wonder if we need a comment in here (and in ansible-test's config) to remind people to keep these two files in sync

This comment has been minimized.

Copy link
@danielmellado

danielmellado Sep 5, 2019

Author Contributor

yeah, I think it'd be helpful not now but in the future. Honestly I'd love for pep8 to be able to map its ignores to a list, pretty much like a requirements.txt

@gundalow

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Please get wait for approval from Matt Clay, incase there is something else we are not aware of.

Add flake8 config file
Even if ansible-test is handling pep8 testing, this commit brings back
and updates the missing flake8 config that went away when removing
legaxy tox.ini.

Signed-off-by: Daniel Mellado <dmellado@redhat.com>
@danielmellado

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

Pull-request updated, HEAD is now 31e6fdf

@danielmellado danielmellado force-pushed the danielmellado:flake8_config branch from 3179cab to 31e6fdf Sep 5, 2019

@ansibot ansibot added the test label Sep 5, 2019

@danielmellado danielmellado requested review from pabelanger and abadger Sep 5, 2019

@abadger

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

I'm not sure we want this for the same reason tox.ini was removed. I don't include config for things we don't use. They get out of date wrt the config for the things we do use and people assume that the lint tool should run without errors when we can't say that because we don't actively maintain it. @mattclay will need to make that decision when he gets back from pto, though.

@danielmellado

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

seems shippable had some issues with docker, closing and reopening to retrigger

@mattclay

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

Configuration should only be included for tools we're using. If there is interest in enforcing flake8 rules in CI then it should be added as a topic to the core meeting agenda so it can be discussed.

We would need to decide whether or not it is useful, especially given existing tests, as well as what configuration to use if it were enabled.

@mattclay

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Closing this since we don't want to add configuration for tools we're not using.

@mattclay mattclay closed this Sep 10, 2019

@sivel sivel removed the needs_triage label Sep 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.