svn commit: r365071 - in head/sys: net net/altq net/route net80211 netgraph netgraph/atm netgraph/atm/ccatm netgraph/atm/sscfu netgraph/atm/sscop netgraph/atm/uni netgraph/bluetooth/common netgraph...

[Mateusz Guzik]

On 9/4/20, Andrew Gallatin <gallatin at cs.duke.edu> wrote:
> On 2020-09-02 22:42, Alexey Dokuchaev wrote:
>
>>> I want to understand which rules have to be followed (and why).
>>
>> In general, FreeBSD code we write should follow style(9); it specifically
>> mentions "do not add whitespace at the end of a line" and "... followed
>> by
>> one blank line" but doesn't go as far as explicitly forbidding multiple
>> consecutive newlines.  To me it's pretty obvious, and while others might
>> have different sens esthe'tique, usually it is lack thereof (no offense)
>> or mere ignorance.
>>
>> ./danfe
>>
>> P.S.  Old-school tools like indent(1) or `uncrustify' were never widely
>> popular, I guess, because they did not possess enough knowledge of the
>> language to always produce correct results.  Perhaps new era tools, like
>> clang-format, could bring this to a whole new level.
>>
>
> I do the upstream sync between the Netflix tree and
> FreeBSD-current about every 3 weeks (unless glebius beats
> me to the punch and does it first :).  I anticipate that
> this blank line sweep will cause lots of conflicts for us.
> I understand this is progress, and I don't object, and I'm
> not asking for a revert, but please understand that cleanups
> like this do have hidden costs.  I expect that other commercial
> entities who contribute to FreeBSD will have the same issue,
> and I also anticipate it will cause problems with MFCs
>
> Rather than doing more sweeps like this, is it possible to
> come up with a clang-format rule that's 95% of style(9), do
> just one more sweep of the tree to apply that rule, add that
> rule as a pre-commit hook, and be done forever with style(9)
> related changes?
>

For starters I completely agree with the need for tooling to prevent
addition of more non-conformant code, but I'm leaving this for someone
else. I don't intend to do any more sweeps (modulo perhaps vfs code).

I would like to note that directories like contrib, zfs, netmap and
others which have known upstream got intentionally skipped during to
not mess with imports. Of course this does not necessary cover all
merges.

Merges regardless of the above (or going the other way) should not be
very problematic though. No design changes are needed, let alone any
testing on account of the above. Worst case this is just minor
annoyance which can be cleaned up while watching The Sopranos with one
eye.

Note that ultimately there is a hidden cost to previous state as well.
What perhaps is not clear from the commit message and wont be seen
unless you scrutinize the diffs, some lines got changed from having
tabs and/or spaces instead of just a newline. This kind of stuff is
jumps at you every time if you have an editor configured to report it
(you can find a vim snipped at the bottom of interested).

That being said, I understand this generated work for some people but
I don't think it consitutes a problem. Also I don't think a heads up
would change anything here. If I was planning more invasive work I
would definitely try to coordinate. Also note the changeset does not
interfere with git blame (unless you want to look at empty lines).

Here is vim snipped I stole from someone several years back:
highlight ExtraWhitespace ctermbg=red guibg=red
autocmd BufWinEnter *.{c,pl,pm} match ExtraWhitespace /^\s*     \|\s\+$/
autocmd InsertEnter *.{c,pl,pm} match ExtraWhitespace /^\s*     \|\s\+\%#\@<!$/
autocmd InsertLeave *.{c,pl,pm} match ExtraWhitespace /^\s*     \|\s\+$/
autocmd BufWinLeave *.{c,pl,pm} call clearmatches()

-- 
Mateusz Guzik <mjguzik gmail.com>