Writing useful comments in Portfiles

[Ryan Schmidt]

Most Portfiles don't need any comments. We especially want to avoid comments that do nothing but state what the code is doing; such comments are counterproductive because they force the reader to read more without giving them more information, and such comments can also easily get out of sync with the code, causing further confusion. But it can be useful to add comments to explain why something is being done, if it is not obvious. Such comments can help you and other developers decide whether the code is still needed when updates are done in the future. I'll give a few examples.

If a Portfile uses "use_autoreconf yes" it is not helpful to add a comment that just says "Run autoreconf" because that's already clear from the code. But it is helpful to explain why autoreconf is being run. For example, the project might not have included a generated configure file; if a comment stated that, then a future developer would know that they can stop running autoreconf if upstream ever starts including a configure file. Or we might be patching configure.ac, in which case a comment lets a future developer know they can stop running autoreconf if that patch is removed. (This is probably the most common reason and is probably what would be assumed if there were no comment.) Or the configure file could have been generated with autotools that were too old that do something wrong on recent macOS, or was generated with intltool and we want to regenerate it with our intltool port's patched intltool.m4.

When you use compiler.blacklist, it's a good idea to add a comment above it explaining why. For example, show the error message you got and add a link to the upstream bug report about that error.

When you specify the C or C++ language standard the port requires, for example "configure.cxx_standard 2014", there's no benefit to adding a comment reading "Require C++14"; the code already makes that clear. The usual reason why you would specify the language version is if the project's code requires that language version, and in that case I wouldn't bother adding a comment. But it might be that the project itself doesn't require that language version but one of its dependencies does and this project uses that dependency's headers. In this case it is useful to add a comment naming the other port that's imposing the newer language version. It lets a future developer know that if that dependency goes away, so does the need for the newer language version, or that if that dependency requires an even newer language version in the future, so will this port.

The C++ language version comment seems to be most pervasive, perhaps because we used to do it by including the cxx11 1.1 portgroup, which blacklisted the compilers that couldn't understand C++11. If a port actually required C++14, then it would additionally blacklist {clang < 602}, or for C++17 it would blacklist {clang < 900} or {clang < 902}. Since there could be any number of reasons why a compiler is blacklisted, there was typically a comment above this blacklist explaining that it was because the project required C++14 or C++17. When we switch this manual blacklisting to using compiler.cxx_standard, both the manual blacklisting of pre-C++14 or pre-C++17 compilers and the comment should be removed. If the port requires additional blacklisting beyond just a language standard, then of course leave that blacklisting in along with a comment explaining its purpose; if it doesn't, then don't forget to remove the inclusion of the compiler_blacklist_versions portgroup.

If you disable a port's universal variant, it's good to say why. Did you encounter an error message? If so, add a comment with the error message or a link to the upstream bug report so that others can follow up on it and maybe help resolve it. Or is it that one of the port's dependencies doesn't have a universal variant? If so, say that in the comment.

I sometimes see comments above epoch lines explaining why they were added. Since a port's epoch can never be decreased, there's never any need to add a comment with an explanation for why an epoch was added. Even if the reasoning for adding the epoch was not sound, it does not matter since it can never be removed or decreased.

I've seen other comments in Portfiles that basically just explain how some standard Portfile variables or procedures work. While I appreciate that these can serve as reminders for maintainers who are still becoming familiar with MacPorts, I'd like to discourage such comments from appearing in Portfiles. Documentation about MacPorts internals belongs in the Guide and manpages.

Portgroups tend to have more complex code, and there are some Portfiles with fairly complex code too. In those cases it can be useful to add comments to explain the trickier parts. But if you can reduce or avoid the need for comments by writing easier-to-understand code, that might be preferable. 

The main thing to avoid, though, is comments that just restate what an obvious line of code is doing.