Regarding:
> commit 7c679d92cd3da8e166c58b0b807bc526035e1f9b
> Author: Nathan Scott <nathans@xxxxxxxxxx>
> Date: Fri Oct 9 14:16:10 2015 +1100
>
> pmmgr: remove unhelpful comments about libpcp and containers
>
> If ever we support nested containers, its going to be pmdaroot
> that needs to handle that, not libpcp. And in general, if you
> are adding comments to code: please don't express a personal /
> sarcastic opinion, and don't say "we should document X" (then
> proceed to not do so). When someone does do these things, the
> comments will be out of date and need fixing too, but are quite
> likely to be forgotten since they're in unrelated code.
This is wrong on several levels. Its tone is inappropriate, as is
enshrining the message in the permanent history of the software
instead of communicating objections by email. But let's move to
substance.
For the record, these are the comments nathans removed:
- // NB: It appears as though the '[?&]container=XXX' suffix is recognized
- // by libpcp on any general hostspec string, incl. ip addresses, hostnames,
- // not just the pcp://* URLs generated by pmDiscoverServices. This should
- // probably be documented.
The first sentence is a simple truth and helps explain neighbouring
code. I cannot imagine anything controversial there. The second
sentence is a todo note / wish-list item, of which we have dozens,
perhaps hundreds in the code. nathans complains that I should have
added a documentation patch instead, but then goes and removes the
todo note and fails to produce any documentation himself (or tracking
artifact of the need). This is a net destruction of information!
- // XXX: what if there are nested containers? oh well, libpcp
- // doesn't handle &container=FOO/BAR
This is another todo-note, since this area of code will likely need to
be revisited should nested containers become supported. nathans
complains that it's pmdaroot rather than libpcp that could be do the
work. Instead of fixing this insignificant nit in the comment, the
note is entirely destroyed.
I cannot see how this commit was appropriate.
- FChE
|