Page MenuHomeFreeBSD

style(9): Default to omitting $FreeBSD$
ClosedPublic

Authored by imp on Feb 3 2022, 5:42 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 24 2022, 10:59 AM
Unknown Object (File)
Dec 24 2022, 10:58 AM
Unknown Object (File)
Dec 24 2022, 10:57 AM
Unknown Object (File)
Dec 24 2022, 10:46 AM
Unknown Object (File)
Dec 24 2022, 10:45 AM
Unknown Object (File)
Dec 24 2022, 10:34 AM
Unknown Object (File)
Dec 15 2022, 12:15 AM

Details

Summary

Advise people to omit $FreeBSD$ unless the code is definitely going to
be merged to stable/12. This strengthens previous statements and is
appropriate now that stable/11 is no longer supported. If people are
wrong and things are unexpected merged to 12, tags can be added before
that merge. No sense adding a tag that will never be expanded and
removed later on the off chance it might wind up in stable/12.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp requested review of this revision.Feb 3 2022, 5:42 AM
rpokala added inline comments.
share/man/man9/style.9
116–117
Code which will definitely be merged into stable/12 should include the
.Li $\&FreeBSD$
tag here.
Otherwise, that tag should be omitted.

(Or maybe may include instead of should include?)

kib added inline comments.
share/man/man9/style.9
52

But do you plan to merge this text to stable/12?

116–117

I agree, the word 'omit' means that the tag have to be included first. May be better say 'No VCS tags should be added', if anything.

Do you also need to update commit hooks ?

I like the change in general.

share/man/man9/style.9
115

I think that 'generally' is unnecessary wordy. I'd drop it.

116–117

+1

Whoops, I hit enter too soon.

It looks like there could be a typo in the proposed commit message: shouldn't "unexpected merged" be "unexpectedly merged"?

0mp requested changes to this revision.Feb 3 2022, 11:25 AM
This revision now requires changes to proceed.Feb 3 2022, 11:25 AM

Do you also need to update commit hooks ?

Nope. There's no checking now, and adding that with enough nuance is a waste of time.

share/man/man9/style.9
52

No. I generally don't MFC style(9) changes. This won't be merged to 12

share/man/man9/style.9
116–117

"New code should only include a
.Li $\&FreeBSD$
tag when it will be merged into stable/12."

maybe. 'may' is definitely the wrong word here, it implies too much discretion. You should do this if you plan on MFCing. And that's the only time you should do this. Not quite a 'must' in RFC-speak, though, imho, which gives no discretion.

tweak based on review
omit another $FreeBSD$

cem added a subscriber: cem.
cem added inline comments.
share/man/man9/style.9
118

I’d use “if” instead of “when.”

@cem: You mean "iff" ? if and only if?

@cem: You mean "iff" ? if and only if?

No, “if.”

share/man/man9/style.9
118

"when" => "if and when" ?

brooks added a subscriber: brooks.

This is a step in the right direction.

IMO, only things managed by mergemaster really need $FreeBSD$ tags. Otherwise adding them on merged code feels like foolish consistency.

pauamma added inline comments.
share/man/man9/style.9
130

While changing this file: maybe change LIB_SCCS to a more recent example? (Did FreeBSD ever use SCCS?)

share/man/man9/style.9
130

I'd rather not change this now....
FreeBSD had this advise because CSRG used SCCS and we imported a lot of stuff from them and needed this advise.
There's still a fair number of these constructs in the tree, so we should document them somewhere.
Though, honestly, it's likely time to consider a sweep of them as well since we don't need them to track upstream anymore, though there are some files with newer versions in lite-2 that we never picked up.

Try again with the wording.
addresses all the comments to date, except Brooks' good point about
mergemaster.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 4 2022, 10:39 PM
This revision was automatically updated to reflect the committed changes.