Skip to content

limit line length to 79 chars #299

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

Closed
wants to merge 1 commit into from
Closed

limit line length to 79 chars #299

wants to merge 1 commit into from

Conversation

rootulp
Copy link
Contributor

@rootulp rootulp commented Feb 3, 2016

No description provided.

@behrtam
Copy link
Contributor

behrtam commented Feb 4, 2016

Just a small tip about our commit message style. The msg should start with the exercise name. https://github.com/exercism/xpython#pull-requests

I'm against pedantically keeping to the 79 limit. While it should definitely not exceed 120 characters, I favour readability over the 79-limit. In this case you split a lambda function because of 2 characters and #300, #301, #302 are equally unappealing to me.

@rootulp
Copy link
Contributor Author

rootulp commented Feb 4, 2016

I think one should have very strong reasons to intentionally contradict PEP8, especially when the majority of the codebase conforms to these standards already. I support the 79 character limit for aesthetic viewing in vim splits. I also think that splitting up long lines aids in readability. In my opinion this:

+        self.assertEqual(inp, accumulate(accumulate(inp, lambda x:
+                         divmod(x, 7)), lambda x: 7 * x[0] + x[1]))

is more readable than this:

-        self.assertEqual(inp, accumulate(accumulate(inp, lambda x: divmod(x, 7)),
-                         lambda x: 7 * x[0] + x[1]))

especially in my editor (left split enforces 79 character limit, right split does not):
screen shot 2016-02-04 at 2 05 48 pm

Having said that, I understand if you don't think the 79 character limit is necessary to enforce and I can close #299, #300, #301, #302. Also thanks for the heads up on commit message style. I'll try to conform to this in the future.

@behrtam
Copy link
Contributor

behrtam commented Feb 5, 2016

First of all we are not really contradicting with PEP8.

Some teams strongly prefer a longer line length. For code maintained exclusively or primarily by a team that can reach agreement on this issue, it is okay to increase the nominal line length from 80 to 100 characters (effectively increasing the maximum length to 99 characters), provided that comments and docstrings are still wrapped at 72 characters.

Right now the maximum line length we have is 92. So maybe we could set our limit to 99 and add this to hour flake8/travis-ci setup.

@kytrinyx
Copy link
Member

kytrinyx commented Feb 5, 2016

I think that line length 99 sounds perfectly reasonable.

behrtam added a commit that referenced this pull request Feb 6, 2016
@behrtam behrtam closed this Feb 6, 2016
@behrtam behrtam mentioned this pull request Mar 19, 2017
@rootulp
Copy link
Contributor Author

rootulp commented Mar 19, 2017

For code maintained exclusively or primarily by a team that can reach agreement on this issue

I don't think we've reached agreement on this issue. I still think it makes sense to conform to the standards in PEP8 unless we have a strong reason not to. Also new contributors must learn that this project has extended the maximum line length from 79 characters to 99 characters. This means they must tune their editors, configure the style checks they run on code, etc.

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

Successfully merging this pull request may close these issues.

3 participants