ubports / crossbuilder

A debian package cross building tool using LXD
13 stars 17 forks source link

Syntax, structure, indent and unneeded logic fixes #60

Open jezek opened 3 years ago

jezek commented 3 years ago
jezek commented 2 years ago

@mardy So what about this PR? In meantime I've got one another approval. It seems, that @violethaze74 is OK (or doesn't care) about the split line merging.

mardy commented 2 years ago

@mardy So what about this PR? In meantime I've got one another approval. It seems, that @violethaze74 is OK (or doesn't care) about the split line merging.

Since this has been waiting for so long and that other contributors haven't voiced their opinion, I guess I'll just pick the changes that I like :-)

jezek commented 2 years ago

...other contributors haven't voiced their opinion, I guess I'll just pick the changes that I like :-)

I'm asking all contributors for this project (@fkaleo , @peat-psuwit , @fredldotme , @UniversalSuperBox , @jld3103 , @lduboeuf ), please read through our conversation (and this one too) and express your opinions and/or vote.

What to vote for? Basicaly it is about line wrapping. My opinion is that that the 3 lines shouldn't be split, because:

What are your opinions about the split lines merging in this PR? What are your personal preferences for splitting lines manually (what is the maximum line length you allow in your code, after which you hard-split lines)?

I'm asking @mardy to wait until the end of January. After this month passes and there is no another vote/opinion (or the vote is a draw), I myself will revert the incriminating split lines merging (but fix the indentation).

Thank you all for your time and understanding.

lduboeuf commented 2 years ago

My opinion: I don't care about conditions writen in one-line. In some cases it is even more readable. Of course not 200 chars long but < 100 is acceptable

jezek commented 2 years ago

@mardy From my perspective I've got an approval from a contributor to merge this, as it is. :wink: What's your understanding of the comment above?

mardy commented 2 years ago

@mardy From my perspective I've got an approval from a contributor to merge this, as it is. wink What's your understanding of the comment above?

I've stated my opinion before, I think it's clear enough. If you want me to merge it, please do not change existing lines to make them longer; that said, I'm certainly not going to start a fight if someone else merges your contribution in its current form. :-)