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

Potential issue with exclude in setup.cfg #130

Open
max-sixty opened this issue Jun 10, 2019 · 2 comments
Open

Potential issue with exclude in setup.cfg #130

max-sixty opened this issue Jun 10, 2019 · 2 comments

Comments

@max-sixty
Copy link

Hi there - we're attempting to have pep8speaks use the flake8 configs in setup.cfg rather than copy them into yaml format.

This works well for the ignore config, but doesn't seem to pick up the exclude config - specifically in the attached PR, we exclude doc, but pep8speaks still runs on that path (those are the only errors it raises)

Here's the PR: pydata/xarray#3010 (comment)

As a separate issue, you're welcome to use xarray in your 'popular projects' or at least in the full list - would you like me to PR us in?

@OrkoHunter
Copy link
Collaborator

Hello @max-sixty ! Thank you for creating the issue and using PEP 8 Speaks.

The config file in the Pull Request looks good to me. However, as documented in the configuration, the bot first searches for the configuration in the base branch of the pull request, which in this case is pydata:master. If no config is found, only then it uses the config of the head branch (proposed changes). This is the reason, PEP 8 Speaks is complaining about the errors in doc because the base branch does not have the exclude configuration in .pep8speaks.yml.

I think there should be a way to force the bot to use the config of the head branch (Maybe by adding a flag [pep8speaks-config-update] in the Pull Request). But as of now, it's not possible to test. The current solution is to update the config in the base branch and test changes on new Pull Requests. (Created #131 as a feature request).

I am keeping this issue open, in case you detect that the config is not working even after being in the base branch. I will be happy to help.

As a separate issue, you're welcome to use xarray in your 'popular projects' or at least in the full list - would you like me to PR us in?

That is awesome! I will go ahead and add it myself. (#132) :)

@max-sixty
Copy link
Author

Thanks @OrkoHunter !

Albeit with a limited perspective, it would be good to read from the PR being merged rather than master; otherwise it makes changing configs more difficult - we can't easily see the impact of proposed changes.

Thanks for an awesome product!

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

No branches or pull requests

2 participants