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

Misc PEP8 cleanup in psf subpackage #925

Merged
merged 4 commits into from
Aug 9, 2019

Conversation

larrybradley
Copy link
Member

@bsipocz
Copy link
Member

bsipocz commented Aug 9, 2019

Would it make sense to tighten the flake8 tests on travis?

@larrybradley
Copy link
Member Author

Yes, good idea. Instead of selecting only a few, we should probably start with everything and only add ignore items as necessary.

@bsipocz
Copy link
Member

bsipocz commented Aug 9, 2019

Originally, as I recall, it only tested for a selected set as some maintainers thought it keeps the bar of entry lower. I'm not sure whether that is true or false.
For astroquery we instead use the pep8speak bot that comments about all but a few ignored ones, but the travis job is running only for a few selected (rather than all but a few ignored) ones. It works nicely, folks keeping cleaning up their PRs based on the bots input. Whether they find it friendly or not, I'm not sure about.

@larrybradley
Copy link
Member Author

The pep8speak bot sounds interesting. I'll check it out. In the meantime I won't update the flake8 rules in travis yet. I'm just running a quick code-check pass before release.

@larrybradley
Copy link
Member Author

Our current flake8 tests are rather minimal.

@larrybradley larrybradley merged commit 05a7264 into astropy:master Aug 9, 2019
@larrybradley larrybradley deleted the psf-pep8 branch August 9, 2019 20:29
@bsipocz
Copy link
Member

bsipocz commented Aug 9, 2019

Before opting in the bot I would ask your local (e.g. mast) folks informally how much they hated to have it on astroquery, is it useful or only annoying.

@larrybradley
Copy link
Member Author

IMHO, the bot looks very useful, especially for beginners. Otherwise, a maintainer will have to go digging in the travis log to see the flake8 failures and then report back. Automation for the win? 🤖 🤖 🤖

@bsipocz
Copy link
Member

bsipocz commented Aug 9, 2019

Yes, I love the bot exactly for those reasons, but I might be in the minority as overall I like the bots 😅

@larrybradley
Copy link
Member Author

I, for one, welcome our new robot overlords.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants