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

pep8speaks.yml being ignored by bot? #944

Closed
eteq opened this issue Aug 15, 2019 · 7 comments · Fixed by #946
Closed

pep8speaks.yml being ignored by bot? #944

eteq opened this issue Aug 15, 2019 · 7 comments · Fixed by #946

Comments

@eteq
Copy link
Member

eteq commented Aug 15, 2019

I admit I don't fully understand how pep8speaks picks its linter, but it seems to me like the pep8speaks.yml at the time of this writing (https://github.com/astropy/photutils/blob/c9a969df4df135ff67107632c2dac488a3a576b3/.pep8speaks.yml) is supposed to be setting the line length limit to 120. However in this issue, the bot was complaining about lines being too long that are 80-some characters:
#745 (comment)
(since the bot updates its comments sometimes, here's a screenshot):
image

Is there maybe a misconfiguration of the bot or something? @bsipocz or @pllim have any idea?

@pllim
Copy link
Member

pllim commented Aug 15, 2019

Ops... maybe flake8 is a staunch Unix terminal fan. Lemme see...

@bsipocz
Copy link
Member

bsipocz commented Aug 15, 2019

PR is merged, but I keep this open until we're sure it indeed worked.

@bsipocz bsipocz reopened this Aug 15, 2019
@bsipocz
Copy link
Member

bsipocz commented Aug 15, 2019

btw the 120 character is inherited from astroquery where we're much relaxed (due to the occasionally longer url strings), I would leave it to Larry to set the preferences here.

@eteq
Copy link
Member Author

eteq commented Aug 15, 2019

FWIW, I am strongly opposed to a strict 80 char limit. But I could be talked down to 100 😉

@bsipocz
Copy link
Member

bsipocz commented Aug 15, 2019

hmm, well, it's up to Larry 🙊
(there are only a mere 38lines that are longer than 80, so that's a totally fine choice here tbh)

@bsipocz
Copy link
Member

bsipocz commented Aug 15, 2019

Closing as it now indeed works, didn't complain about the 84 char long lines in #947

@bsipocz bsipocz closed this as completed Aug 15, 2019
@pllim
Copy link
Member

pllim commented Aug 15, 2019

Phew!

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

Successfully merging a pull request may close this issue.

3 participants