Paths

Table of Contentst

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

Authored by imp on Feb 3 2022, 5:42 AM.

Details

Reviewers

0mp
kib
• hselasky
cem
brooks
erj
rpokala

Group Reviewers

manpages

Commits

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

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.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp created this revision.Feb 3 2022, 5:42 AM

Herald added a reviewer: manpages. · View Herald TranscriptFeb 3 2022, 5:42 AM

imp requested review of this revision.Feb 3 2022, 5:42 AM

Harbormaster completed remote builds in B44267: Diff 102308.Feb 3 2022, 5:42 AM

rpokala added a subscriber: rpokala.Feb 3 2022, 6:02 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 a subscriber: kib.Feb 3 2022, 10:11 AM

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

In D34152#772429, @hselasky wrote:

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

imp added inline comments.Feb 3 2022, 2:02 PM

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$

Harbormaster completed remote builds in B44272: Diff 102320.Feb 3 2022, 2:28 PM

kib accepted this revision.Feb 3 2022, 2:43 PM

Looks good.

cem accepted this revision.Feb 3 2022, 2:57 PM

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?

In D34152#772589, @hselasky wrote:

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

No, “if.”

rpokala added inline comments.Feb 3 2022, 4:48 PM

share/man/man9/style.9
118	"when" => "if and when" ?

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.

erj accepted this revision.Feb 3 2022, 11:19 PM

pauamma_gundo.com added a subscriber: pauamma_gundo.com.Feb 4 2022, 12:58 AM

pauamma_gundo.com 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?)

imp added inline comments.Feb 4 2022, 1:37 AM

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.