[all][tc] Lets talk about Flake8 E501
Hello All,
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line.
How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK.
The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
Here is an example of one extreme, a diff of two files maxing out at 119 characters
https://review.opendev.org/c/opendev/sandbox/+/843697/1..2
Thank you for your time and I am looking forward to this conversation :)
-- Miro Tomaska irc: mtomaska Red Hat
On 2022-05-27 21:24:32 -0500 (-0500), Miro Tomaska wrote:
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit.
[...]
This is a choice individual projects can make; the TC hasn't demanded all projects follow a specific coding style.
That said, I personally do all my coding and code review in 80-column text terminals, so any lines longer that that end up wrapping and making indentation harder to follow. There have been numerous studies to suggest that shorter lines are easier for people to read and comprehend, whether it's prose or source code, and the ideal lengths actually end up being around 50-75 characters. There's a reason wide-format print media almost always breaks text into multiple columns: The eyes tend to get lost as you scan across longer lines of content. (As an aside, you'll probably notice I've set my MUA to wrap all lines at 68 characters.)
Just my opinion, but I do really appreciate when projects keep source code files wrapped at 79 columns.
On Sat, May 28, 2022 at 2:29 PM Jeremy Stanley fungi@yuggoth.org wrote:
On 2022-05-27 21:24:32 -0500 (-0500), Miro Tomaska wrote:
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit.
[...]
This is a choice individual projects can make; the TC hasn't demanded all projects follow a specific coding style.
That said, I personally do all my coding and code review in 80-column text terminals, so any lines longer that that end up wrapping and making indentation harder to follow.
Similarly, I usually split my vim in 2 halves, around 90 characters fit into each half.
Dmitry
There have been numerous studies to suggest that shorter lines are easier for people to read and comprehend, whether it's prose or source code, and the ideal lengths actually end up being around 50-75 characters. There's a reason wide-format print media almost always breaks text into multiple columns: The eyes tend to get lost as you scan across longer lines of content. (As an aside, you'll probably notice I've set my MUA to wrap all lines at 68 characters.)
Just my opinion, but I do really appreciate when projects keep source code files wrapped at 79 columns. -- Jeremy Stanley
On Mon, May 30, 2022 at 3:46 PM Dmitry Tantsur dtantsur@redhat.com wrote:
On Sat, May 28, 2022 at 2:29 PM Jeremy Stanley fungi@yuggoth.org wrote:
On 2022-05-27 21:24:32 -0500 (-0500), Miro Tomaska wrote:
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit.
[...]
This is a choice individual projects can make; the TC hasn't demanded all projects follow a specific coding style.
That said, I personally do all my coding and code review in 80-column text terminals, so any lines longer that that end up wrapping and making indentation harder to follow.
Similarly, I usually split my vim in 2 halves, around 90 characters fit into each half.
Likewise; I'm still a big fan of shorter line lengths, so I'd prefer 79 columns too.
Alex.
---- On Fri, 27 May 2022 21:24:32 -0500 Miro Tomaska mtomaska@redhat.com wrote ----
Hello All, This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line. How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK. The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
This is a good point and having such in-consistency will be more problems than having it more than 79 char for the reason you mentioned above. And I do not think we will be able to convert all the existing code to the new limit.
I feel more comfortable with 79 char when I review on a small or large screen. If we end up scrolling horizontally then it is definitely not good. Log files are good examples of it.
For long line/var name, we do split line due to the 79 char limit but I do not think that is more cases in our code, maybe ~1% of our existing code?
I think keeping consistency in code is important which is what flake8/pep8 checks are all about.
-gmann
Here is an example of one extreme, a diff of two files maxing out at 119 characters https://review.opendev.org/c/opendev/sandbox/+/843697/1..2 Thank you for your time and I am looking forward to this conversation :) -- Miro Tomaskairc: mtomaskaRed Hat
On Sat, 2022-05-28 at 11:04 -0500, Ghanshyam Mann wrote:
---- On Fri, 27 May 2022 21:24:32 -0500 Miro Tomaska mtomaska@redhat.com wrote ----
Hello All, This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line. How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK. The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
This is a good point and having such in-consistency will be more problems than having it more than 79 char for the reason you mentioned above. And I do not think we will be able to convert all the existing code to the new limit.
we would not convert it that woudl break git blame. but i dont see the consonsitnce being an issue we would just update teh older code whenever it was next touched if needed. its not as if all lines are 79 chars today the code with varies anyway
I feel more comfortable with 79 char when I review on a small or large screen. If we end up scrolling horizontally then it is definitely not good. Log files are good examples of it.
even on my phone which i alsmost never use use 80 dose not really help much as i would generally put my phone in landscape in that case and i would have more then enough space for 2 120 char colums side by side. when i used ot code on my 13 inch dell xps i also used ot have multiple terminal/sbuffer side by side. granted it was a 4k dispaly but even on my work laptop which is only 1080p or my ipad generally have room for 2 150 ish cahrater terminals
For long line/var name, we do split line due to the 79 char limit but I do not think that is more cases in our code, maybe ~1% of our existing code?
it deffinetly more common then that. i know i at least often end up rewriging my code locally befor ei push it to fit in the currently limit on most patches i write. i stongly belive our current file limits are reduceing readblity not helping it. i however dont want to break git blames usfully ness so any cahnge we woudl make in the fucutre would have to not touch the exsitng code and be gradual.
like for example swaping to black i would be strongly against since it fundemtaly chagnes how code is formated and breaks git blame but i was infavor of adding autopep8 which had minimal change to nova when we added it since it only fixes pep8 issue and does not reformat all the code.
if we were to adopt a new longer line limit i would hope we ensure that we only use it for new code an let the code organcialy convert. im not actully saying we shoudl change our line limits on a per project or comunity wide basis just that outside fo readbilyt there is a maintainablity aspect to consider.
with that said i really prefer when tools enforce style not humans so any change would have to be enforcable by tooling/ci too or its a non starter in my book. flake8 does htave teh ablity to set teh line lenght https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-max-li... and tools like autopep8 can also consume that but not all ides or editors will.
I think keeping consistency in code is important which is what flake8/pep8 checks are all about.
it is but i dont think applies ot lines that are shorter the the new line lenght. i.e. removeign lines that were split and replacing them with unsplit lines that now fit. that is something that should only be done if you are modifying that line for a different reason.
-gmann
Here is an example of one extreme, a diff of two files maxing out at 119 characters https://review.opendev.org/c/opendev/sandbox/+/843697/1..2 Thank you for your time and I am looking forward to this conversation :) -- Miro Tomaskairc: mtomaskaRed Hat
On Mon, 2022-05-30 at 11:57 +0100, Sean Mooney wrote:
On Sat, 2022-05-28 at 11:04 -0500, Ghanshyam Mann wrote:
---- On Fri, 27 May 2022 21:24:32 -0500 Miro Tomaska mtomaska@redhat.com wrote ----
Hello All, This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line. How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK. The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
This is a good point and having such in-consistency will be more problems than having it more than 79 char for the reason you mentioned above. And I do not think we will be able to convert all the existing code to the new limit.
we would not convert it that woudl break git blame.
This isn't true. You can configure a '.git-blame-ignore-revs' file that will cause git-blame to ignore particular revision(s). The Django community recently used this feature when they ran black over the Django codebase [2] and I added one such file to sqlalchemy a few weeks back for the same reason [3] (they did their black'ifying a few years ago). GitHub will now parse these files also [4] though I don't know if gitea has this functionality yet. In any case, my point is there are certainly valid arguments against larger wrapping width but breaking git-blame isn't really one of them anymore.
Stephen
[1] https://stackoverflow.com/questions/34957237/ [2] https://news.ycombinator.com/item?id=30258530 [3] https://github.com/sqlalchemy/sqlalchemy/commit/27828d668dd [4] https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-be...
but i dont see the consonsitnce being an issue we would just update teh older code whenever it was next touched if needed. its not as if all lines are 79 chars today the code with varies anyway
I feel more comfortable with 79 char when I review on a small or large screen. If we end up scrolling horizontally then it is definitely not good. Log files are good examples of it.
even on my phone which i alsmost never use use 80 dose not really help much as i would generally put my phone in landscape in that case and i would have more then enough space for 2 120 char colums side by side. when i used ot code on my 13 inch dell xps i also used ot have multiple terminal/sbuffer side by side. granted it was a 4k dispaly but even on my work laptop which is only 1080p or my ipad generally have room for 2 150 ish cahrater terminals
For long line/var name, we do split line due to the 79 char limit but I do not think that is more cases in our code, maybe ~1% of our existing code?
it deffinetly more common then that. i know i at least often end up rewriging my code locally befor ei push it to fit in the currently limit on most patches i write. i stongly belive our current file limits are reduceing readblity not helping it. i however dont want to break git blames usfully ness so any cahnge we woudl make in the fucutre would have to not touch the exsitng code and be gradual.
like for example swaping to black i would be strongly against since it fundemtaly chagnes how code is formated and breaks git blame but i was infavor of adding autopep8 which had minimal change to nova when we added it since it only fixes pep8 issue and does not reformat all the code.
if we were to adopt a new longer line limit i would hope we ensure that we only use it for new code an let the code organcialy convert. im not actully saying we shoudl change our line limits on a per project or comunity wide basis just that outside fo readbilyt there is a maintainablity aspect to consider.
with that said i really prefer when tools enforce style not humans so any change would have to be enforcable by tooling/ci too or its a non starter in my book. flake8 does htave teh ablity to set teh line lenght https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-max-li... and tools like autopep8 can also consume that but not all ides or editors will.
I think keeping consistency in code is important which is what flake8/pep8 checks are all about.
it is but i dont think applies ot lines that are shorter the the new line lenght. i.e. removeign lines that were split and replacing them with unsplit lines that now fit. that is something that should only be done if you are modifying that line for a different reason.
-gmann
Here is an example of one extreme, a diff of two files maxing out at 119 characters https://review.opendev.org/c/opendev/sandbox/+/843697/1..2 Thank you for your time and I am looking forward to this conversation :) -- Miro Tomaskairc: mtomaskaRed Hat
On Mon, 2022-05-30 at 12:23 +0100, Stephen Finucane wrote:
On Mon, 2022-05-30 at 11:57 +0100, Sean Mooney wrote:
On Sat, 2022-05-28 at 11:04 -0500, Ghanshyam Mann wrote:
---- On Fri, 27 May 2022 21:24:32 -0500 Miro Tomaska mtomaska@redhat.com wrote ----
Hello All, This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line. How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK. The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
This is a good point and having such in-consistency will be more problems than having it more than 79 char for the reason you mentioned above. And I do not think we will be able to convert all the existing code to the new limit.
we would not convert it that woudl break git blame.
This isn't true. You can configure a '.git-blame-ignore-revs' file that will cause git-blame to ignore particular revision(s). The Django community recently used this feature when they ran black over the Django codebase [2] and I added one such file to sqlalchemy a few weeks back for the same reason [3] (they did their black'ifying a few years ago). GitHub will now parse these files also [4] though I don't know if gitea has this functionality yet. In any case, my point is there are certainly valid arguments against larger wrapping width but breaking git-blame isn't really one of them anymore.
is this new as that has been a sticking point to me making nova's automatic code formating stricter/better i have some patches locally to have pre-commit fix more things automtically but i was holding off on them as one of the fixes was normalising your use of single and double quotes.
git blame was why it took like 3 years to finally get even basic auto code formating to land.
i think there are are more valid argument in favor of longer lines then against but git blame had always been the main sticking point that kills any efforts to move to code formating been doen exclivivly via tooling in the past.
backports is the other but i can live with fixing the formatign for older brnahces wehn we backport once when going to yoga if it means we can automate formating on master.
this is a tangent to the can we use 120 lines or not question.
Stephen
[1] https://stackoverflow.com/questions/34957237/ [2] https://news.ycombinator.com/item?id=30258530 [3] https://github.com/sqlalchemy/sqlalchemy/commit/27828d668dd [4] https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-be...
but i dont see the consonsitnce being an issue we would just update teh older code whenever it was next touched if needed. its not as if all lines are 79 chars today the code with varies anyway
I feel more comfortable with 79 char when I review on a small or large screen. If we end up scrolling horizontally then it is definitely not good. Log files are good examples of it.
even on my phone which i alsmost never use use 80 dose not really help much as i would generally put my phone in landscape in that case and i would have more then enough space for 2 120 char colums side by side. when i used ot code on my 13 inch dell xps i also used ot have multiple terminal/sbuffer side by side. granted it was a 4k dispaly but even on my work laptop which is only 1080p or my ipad generally have room for 2 150 ish cahrater terminals
For long line/var name, we do split line due to the 79 char limit but I do not think that is more cases in our code, maybe ~1% of our existing code?
it deffinetly more common then that. i know i at least often end up rewriging my code locally befor ei push it to fit in the currently limit on most patches i write. i stongly belive our current file limits are reduceing readblity not helping it. i however dont want to break git blames usfully ness so any cahnge we woudl make in the fucutre would have to not touch the exsitng code and be gradual.
like for example swaping to black i would be strongly against since it fundemtaly chagnes how code is formated and breaks git blame but i was infavor of adding autopep8 which had minimal change to nova when we added it since it only fixes pep8 issue and does not reformat all the code.
if we were to adopt a new longer line limit i would hope we ensure that we only use it for new code an let the code organcialy convert. im not actully saying we shoudl change our line limits on a per project or comunity wide basis just that outside fo readbilyt there is a maintainablity aspect to consider.
with that said i really prefer when tools enforce style not humans so any change would have to be enforcable by tooling/ci too or its a non starter in my book. flake8 does htave teh ablity to set teh line lenght https://flake8.pycqa.org/en/latest/user/options.html#cmdoption-flake8-max-li... and tools like autopep8 can also consume that but not all ides or editors will.
I think keeping consistency in code is important which is what flake8/pep8 checks are all about.
it is but i dont think applies ot lines that are shorter the the new line lenght. i.e. removeign lines that were split and replacing them with unsplit lines that now fit. that is something that should only be done if you are modifying that line for a different reason.
-gmann
Here is an example of one extreme, a diff of two files maxing out at 119 characters https://review.opendev.org/c/opendev/sandbox/+/843697/1..2 Thank you for your time and I am looking forward to this conversation :) -- Miro Tomaskairc: mtomaskaRed Hat
Le sam. 28 mai 2022 à 04:36, Miro Tomaska mtomaska@redhat.com a écrit :
Hello All,
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line.
How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK.
The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
Here is an example of one extreme, a diff of two files maxing out at 119 characters
https://review.opendev.org/c/opendev/sandbox/+/843697/1..2
Thank you for your time and I am looking forward to this conversation :)
My personal opinion is while I understand the reasoning behind, my concern is about the community support. We don't have a lot of contributors at the moment, and if we would like to modify the default limit, I'm pretty sure it would take a lot of time for folks at least reviewing some changes that are not really priorities...
Also, as said by some folks, changing the default would also create some problems for some of our contributors and eventually, if some projects would accept 119 characters while some others not, it would also be a new community difference between projects... (and yet again some new nit that new contributors need to understand)
Sorry, -1 for those reasons.
--
Miro Tomaska irc: mtomaska Red Hat
Thank you all for responding and sharing your thoughts. It seems that although the longer limit might be nice it will bring more complications than we want to. For that reason we can consider this discussion as closed. That is, there will be no chances.
Thanks again Miro
On Tue, May 31, 2022 at 8:22 AM Sylvain Bauza sbauza@redhat.com wrote:
Le sam. 28 mai 2022 à 04:36, Miro Tomaska mtomaska@redhat.com a écrit :
Hello All,
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line.
How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK.
The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
Here is an example of one extreme, a diff of two files maxing out at 119 characters
https://review.opendev.org/c/opendev/sandbox/+/843697/1..2
Thank you for your time and I am looking forward to this conversation :)
My personal opinion is while I understand the reasoning behind, my concern is about the community support. We don't have a lot of contributors at the moment, and if we would like to modify the default limit, I'm pretty sure it would take a lot of time for folks at least reviewing some changes that are not really priorities...
Also, as said by some folks, changing the default would also create some problems for some of our contributors and eventually, if some projects would accept 119 characters while some others not, it would also be a new community difference between projects... (and yet again some new nit that new contributors need to understand)
Sorry, -1 for those reasons.
--
Miro Tomaska irc: mtomaska Red Hat
Hi,
Dnia wtorek, 31 maja 2022 15:21:50 CEST Sylvain Bauza pisze:
Le sam. 28 mai 2022 à 04:36, Miro Tomaska mtomaska@redhat.com a écrit :
Hello All,
This is probably going to be a hot topic but I was wondering if the community ever considered raising the default 79 characters line limit. I have seen some places where even a very innocent line of code needs to be split into two lines. I have also seen some code where I feel like variable names were abbreviated on purpose to squeeze everything into one line.
How does the community feel about raising the E501 limit to 119 characters? The 119 character limit is the second most popular limit besides the default one. It's long enough to give a developer enough room for descriptive variables without being forced to break lines too much. And it is short enough for a diff between two files to look OK.
The only downside I can see right now is that it's not easy to convert an existing code. So we will end up with files where the new code is 79+ characters and the "old" code is <=79. I can also see an argument where someone might have trouble reviewing a patch on a laptop screen (assuming standard 14" screen) ?
Here is an example of one extreme, a diff of two files maxing out at 119 characters
https://review.opendev.org/c/opendev/sandbox/+/843697/1..2
Thank you for your time and I am looking forward to this conversation :)
My personal opinion is while I understand the reasoning behind, my concern is about the community support. We don't have a lot of contributors at the moment, and if we would like to modify the default limit, I'm pretty sure it would take a lot of time for folks at least reviewing some changes that are not really priorities...
Also, as said by some folks, changing the default would also create some problems for some of our contributors and eventually, if some projects would accept 119 characters while some others not, it would also be a new community difference between projects... (and yet again some new nit that new contributors need to understand)
Sorry, -1 for those reasons.
I don't have strong opinion about it but I see that e.g. You are saying that this should be changed (or stay like it is now) globally for all projects but earlier fungi said that there wasn't such requirement from e.g. TC to have this configured in same way for all projects. So IIUC every project can actually change it on their own if team wants to do that.
Personally I agree with Your point that it should be done for all projects in the same way so maybe TC should have some guidance/rules for that to make sure all projects will follow the same pattern. Wdyt?
--
Miro Tomaska irc: mtomaska Red Hat
On 2022-06-01 22:51:53 +0200 (+0200), Slawek Kaplonski wrote: [...]
Personally I agree with Your point that it should be done for all projects in the same way so maybe TC should have some guidance/rules for that to make sure all projects will follow the same pattern. Wdyt?
Given that the vast majority of the code in OpenStack is consistent on this point even in the absence of any requirement on the part of the TC, I take it as an indication that having the TC make a rule about it would be a redundant time-sink when there's plenty else for them to be working on.
participants (9)
-
Alex Kavanagh
-
Dmitry Tantsur
-
Ghanshyam Mann
-
Jeremy Stanley
-
Miro Tomaska
-
Sean Mooney
-
Slawek Kaplonski
-
Stephen Finucane
-
Sylvain Bauza