@emaste thanks for <https://github.com/freebsd/freebsd-src/commit/97beb0c9116f312c1820adb94455dc3b9a157135> – <https://github.com/freebsd/freebsd-src/blob/main/CONTRIBUTING.md> is certainly easier to digest.
A handful of minor questions/issues …
Discussion
Loading...
Post
<https://github.com/freebsd/freebsd-src/blob/main/CONTRIBUTING.md#style> mentions the one-sentence-per-line rule for manual pages, however:
a) there's no such rule in mdoc(7) <https://man.freebsd.org/cgi/man.cgi?query=mdoc&sektion=7&manpath=freebsd-current> or style.mdoc(5) <https://man.freebsd.org/cgi/man.cgi?query=style.mdoc&sektion=5&manpath=freebsd-current>; and
b) I frequently see multiple lines for a single sentence – is this simply because authors and reviewers happily ignore a rule?
A random pick, from a very recently changed page:
– the longest line is 78 characters.
FreeBSD style aside, Debian man-pages(7) suggests a source code line length of no more than about 75 characters wherever possible: <https://manpages.debian.org/trixie/manpages/man-pages.7.en.html#Conventions_for_source_file_layout>.
I generally find FreeBSD’s style(9) to be exactly the opposite of the coding conventions you’d adopt to make bugs visible. Most man pages I’ve read have been wrapped on some arbitrary boundary and it makes diffs really hard to review.
For non-code text, I always use a one-sentence-per-line rule. It makes review much easier and means that line-diff tools (which, for better or worse, are the tools we have) work nicely.
line-diff tools (which, for better or worse, are the tools we have)
git diff --color-words is a tool that we have, and an extremely useful one for making prose diffs more readable, FWIW. (Even if sentence-per-line formatted, but especially so if not.)
Which is fine when you have it, but web review UIs and patches in emails are less clear (GitHub does highlight changed words).
@david_chisnall I have only one minor criticism of GitHub's highlighting, and I don't know whether it is (or should be) fixable.
In cases such as the one pictured below, it's necessary to scroll/page down hugely to bring a tiny change into sight.
In other words, 'Show Diff' shows much, much more than the diff.
A wild guess: maybe specific to AsciiDoc in GitHub, or the FreeBSD Documentation Project's use of AsciiDoc in cases such as this.
2 media
As a concrete example: many years ago it was a struggle to make a change that allowed putting braces around single-statement bodies of if statements. It was previously disallowed entirely, now it is optional. This is a common source of bugs when people make changes to a file and add another statement to a conditional, or simply misread the behaviour. Even when they do it correctly, it makes the diffs harder to review because the lines above and blow are changed to add braces. This composes particularly badly with the rule that open braces go on the line with the if statement (ignoring the other reasons that is bad) because it means now the s up in the diff and reviewers need to read it to see that the only change is adding a brace at the end. All of this increases cognitive load that could be better spent seeing if the logic is correct.