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

Use a max line length of 80 universally, and allowlist docstrings with # noqa: E501 #1532

Closed
ianstenbit opened this issue Mar 16, 2023 · 5 comments · Fixed by #1552
Closed

Comments

@ianstenbit
Copy link
Contributor

See conversation in #1509.

This should essentially just require that we switch our linter config to universally enforce 80 chars/line, and add the # noqa: E501 tag to all docstrings > 80 chars wide.

@ID6109
Copy link
Contributor

ID6109 commented Mar 16, 2023

I'd like to take this up @ianstenbit

@haifeng-jin
Copy link
Contributor

For long lines of URLs in the docstrings, we should tag them with # noqa: E501.

For other types of long lines that can be adjusted, we should just adjust them. They exist in the codebase because we did not enforce 80 char for a while. So some PRs with other long lines also got merged.

The PR should also change the max-line-length to 80 from 200 in setup.cfg.

@ID6109
Copy link
Contributor

ID6109 commented Mar 17, 2023

The max-line-length is already set to 80. Should the max-doc-length be changed from 200 to 80? W505 errors will be introduced then.

@haifeng-jin
Copy link
Contributor

After #1509 is merged, the max-line-length will be 200 and max-doc-length will be removed.

@ID6109
Copy link
Contributor

ID6109 commented Mar 18, 2023

I'll submit a PR by tomorrow

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