Mk/bsd.options.mk: Update comments.
Needs ReviewPublic
Actions

Authored by zirias on Dec 23 2022, 2:41 PM.

Details

Reviewers

Group Reviewers

Summary

Update the comments documenting OPT_USE and similar to be in line with
porter's handbook. In the handbook, the keys used in these are shown in
lowercase, see for example
https://docs.freebsd.org/en/books/porters-handbook/book/#options-use

The comments showed them in uppercase so far.

Diff Detail

Repository

rP FreeBSD ports repository

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 48872
Build 45761: arc lint + arc unit

Event Timeline

zirias created this revision.Dec 23 2022, 2:41 PM

Herald added a reviewer: portmgr. · View Herald TranscriptDec 23 2022, 2:41 PM

Herald added a subscriber: mat. · View Herald Transcript

zirias requested review of this revision.Dec 23 2022, 2:41 PM

Harbormaster completed remote builds in B48872: Diff 114507.Dec 23 2022, 2:41 PM

It seems the uppercase form is more popular so far:

# git grep -l '_USE=.*xorg=' -- '*/*/Makefile' | wc -l
      55
# git grep -l '_USE=.*XORG=' -- '*/*/Makefile' | wc -l
     182
# git grep -l '_USE=.*qt=' -- '*/*/Makefile' | wc -l
      28
# git grep -l '_USE=.*QT=' -- '*/*/Makefile' | wc -l
     111
# git grep -l '_USE=.*kde=' -- '*/*/Makefile' | wc -l
       5
# git grep -l '_USE=.*KDE=' -- '*/*/Makefile' | wc -l
      18
# git grep -l '_USE=.*gnome=' -- '*/*/Makefile' | wc -l
      66
# git grep -l '_USE=.*GNOME=' -- '*/*/Makefile' | wc -l
     229

So, we could discuss updating the porter's handbook instead. But I personally think lowercase comes more natural for the *content* of a variable, and the conversion to uppercase when applying it is there for a reason.

In D37861#860185, @zirias wrote:
It seems the uppercase form is more popular so far:
# git grep -l '_USE=.*xorg=' -- '*/*/Makefile' | wc -l
      55
# git grep -l '_USE=.*XORG=' -- '*/*/Makefile' | wc -l
     182
# git grep -l '_USE=.*qt=' -- '*/*/Makefile' | wc -l
      28
# git grep -l '_USE=.*QT=' -- '*/*/Makefile' | wc -l
     111
# git grep -l '_USE=.*kde=' -- '*/*/Makefile' | wc -l
       5
# git grep -l '_USE=.*KDE=' -- '*/*/Makefile' | wc -l
      18
# git grep -l '_USE=.*gnome=' -- '*/*/Makefile' | wc -l
      66
# git grep -l '_USE=.*GNOME=' -- '*/*/Makefile' | wc -l
     229
So, we could discuss updating the porter's handbook instead. But I personally think lowercase comes more natural for the *content* of a variable, and the conversion to uppercase when applying it is there for a reason.

Majority=uppercase, maybe that should be the documented way?

I personally fail to see any benefit from the lower-case notation.

No other opionions?

Well, my interpretation was that UPPERCASE is reserved to variable *names*. Of course that's debatable here, as these variables just "aggregate" other variables ...

Someone added the case conversion when expanding them, so obviously had something in mind ;-) But of course we could also just drop that (simplifying the code) and update the porters' handbook instead.

mat added inline comments.Feb 15 2023, 9:10 AM

Mk/bsd.options.mk
146–159	Maybe be explicit in the fact that foo will be uppercased, I know the example says so, but it is probably better to write it down so that nobody can say this was unexpected. This is probably why I wrote FOO=bar in the first case.

zirias added inline comments.Mar 9 2023, 11:42 AM

Mk/bsd.options.mk
146–159	Makes sense, if uppercasing is actually kept. Could also drop uppercasing, leave these comments as is and update the porters handbook instead. Still unsure which one is the better way, but consistency would be nice.

Revision Contents
Changeset List

Path

Size

Mk/

bsd.options.mk

14 lines

Diff 114507

View Options

Mk/bsd.options.mk: Update comments.Needs ReviewPublicActions