Open Stack

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 at 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-beta/
> 
> > 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-line-length
> > 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
> > > 
> > 
> > 
>