style.9: Encourage style changes when doing significant modifications

[olce]

Summary

The rule of allowing style changes when about 50% or more of a file (or
group of files), coupled with the advice of avoiding stylistic changes,
could be interpreted as forbidding most style changes, even in heavily
modified functions.

In order to rule out that interpretation and ease transition towards our
prescribed style:

1. Clarify that avoiding stylistic changes concerns only "standalone" ones.
2. Actually encourage changing the style, and extend the cases where it is explicitly allowed to do so to any single logical unit as little as a function, keeping the existing ~50% of modified code as a rule of thumb.

When point 2 above applies, encourage to commit pure style changes
separately, and to add style-only commits to '.git-blame-ignore-revs'.

Add a specific note ruling out "horizontal" style changes spanning
unrelated directories in the whole tree, which make the MFC process more
difficult.

While here, be slightly more stringent on new kernel code.

While here, regroup the paragraphs talking about style, and put them at
the end (before the recent C++ section).

While here, rephrase the requirement on third-party maintained code to
be slightly less stringent.

[jhb]

share/man/man9/style.9

872

I would not say it's hard on the repo per se, but instead, it can make git blame harder to use (though there is some mitigation for that), and it can be disruptive to downstream consumers.

877

s/can/should/? Do we also want to document the git blame workaround here?

[imp]

share/man/man9/style.9

872

Yea, we have mechanisms to cope with blame, but downstream users have given the feedback repeatedly that style only changes are useless to them: they create more work w/o giving any value.

Maybe it's time to switch the focus here. Purely stylistic changes are hard for our ecosystem to swallow: they create more work for our downstreams and provide little to no benefit. They can also make tracing functional chanages harder.

878

We also have some files that we've decided, for whatever reason, to change the style in place: as we add new functions, we use the new style (often accidentally at first, because mike smith code is 4-space code and adding functions to that emacs automatically does 8-space-with-a-tab formatting). so in files like this, we've taken the logical group approach: I had 5 new functions and reformat 4 existing ones in a commit before the new function commit.

[olce]

• Amend in the direction of feedbacks.
• Move the whole block of text to the end of the section (but still before the recent C++ section).

[olce]

Ping?

[imp]

I'm mostly OK with this, but I'd be happier if we could address horizontal vs vertical changes more explicitly than 'pure style changes are discouraged'

share/man/man9/style.9

910

I'd say 'about half' instead of 'about 50%' here. It's the same thing, but a fuzzier measure.

925

I'd find some way to say that 'horizontal' style changes should be avoided. These are changes across the whole tree in directories that are unrelated (in that they aren't merged together). these make it harder to MFC since the automated tooling will pull them in. But since they are spread over so many directories, some may be up-to-date with MFC and others not and so conflicts are more likely.

[obrien]

share/man/man9/style.9

872

Actually for Juniper having things in improper style has caused non-FreeBSD committers to copy the wrong style - because that's what's there and they don't know better. The more consistent our style, the more consistent our non-committers developers are.

[emaste]

share/man/man9/style.9

872

Never-ending style(9) churn is a big disruption, but we can look at lldb's experience for an illustrative example. lldb used to have a style that was inconsistent with the rest of llvm and also was not consistently applied within lldb itself. They did one massive clang-format run with lots of advance notice, and there is no longer any significant effort spent on keeping it in style.

Commit: https://github.com/llvm/llvm-project/commit/b9c1b51e45b845debb76d8658edabca70ca56079

I've been running experiments with clang-format on occasion, with one example in D30260 for reference.

899

[olce]

Cater to comments.

share/man/man9/style.9

872

Actually for Juniper having things in improper style has caused non-FreeBSD committers to copy the wrong style - because that's what's there and they don't know better. The more consistent our style, the more consistent our non-committers developers are.

I agree, that's why we should strive to gradually converge to style(9), hence encouraging such changes in some cases where the drawbacks are reduced.

872

Never-ending style(9) churn is a big disruption, but we can look at lldb's experience for an illustrative example. lldb used to have a style that was inconsistent with the rest of llvm and also was not consistently applied within lldb itself. They did one massive clang-format run with lots of advance notice, and there is no longer any significant effort spent on keeping it in style.

Commit: https://github.com/llvm/llvm-project/commit/b9c1b51e45b845debb76d8658edabca70ca56079

I've been running experiments with clang-format on occasion, with one example in D30260 for reference.

I think this approach would be workable for downstreams if we can indeed provide a tool that formats all files in a way that reproduces almost exactly the reformatting we would do in such a mega-commit. Looking at the revision in reference, it does not seem we are at this point yet, and to reach it clang-format on its own may never be sufficient (so we would have to develop something, perhaps based on clang-format or some other existing formatter).

[ziaee]

share/man/man9/style.9

919

I have never heard of this. We should put this in comitters guide.

[emaste]

share/man/man9/style.9

914

or even is going to be modified / is expected to be modified

[olce]

"modified" => "going to be modified"