Nathan Scott <nathans@xxxxxxxxxx> writes:
> [...] Hmm, this re-revert is NQR. Its also a bit surprising that
> you ah weren't informed it had been previously reverted and that it
> was not generally agreed with.
If the standard is "general agreement", then leave revert wars to
wikipedia, not here. Disagreements should not be decided by one
party's fiat, but by group discussion.
> index 524d2d1..bf7ade6 100644
> --- a/src/pmmgr/pmmgr.cxx
> +++ b/src/pmmgr/pmmgr.cxx
> @@ -527,6 +527,11 @@ pmmgr_job_spec::poll()
> ++it2) {
> // XXX: presuming that the container name is safe & needs
> no escape;
> // on docker, this is ok because the container id is a
> long hex string.
> + //
> + // 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.
>
>
> I suggested these 'actually' be documented properly in man pages, not just
> 'probably', but that was ignored. Someone else updated the man page docs
> in the meantime, and this comment was appropriately dropped.
Then the right thing to do would have been to update the comment by
removing its last sentence, not call it "reverting" anything. It was
obviously correct at the time it was written, and the rest of the note
is still correct. Surely you're not suggesting that a factual, 100%
true description of behavior is inappropriate.
> known_targets[subtarget_hostid] = subtarget_spec;
> }
> }
> + // XXX: what if there are nested containers? oh well, libpcp
> + // doesn't handle &container=FOO/BAR
> }
>
>
> Several times it was explained that this comment doesn't make sense, libpcp
> does not (and should not) know anything about containers, let alone these
> theoretical nested containers. It was suggested to change it (mention the
> pmdaroot involvement, maybe move to pmdaroot if its really necessary), but
> that was not done either.
If you wield the power of unilateral reversion, you also wield the
power of correcting (what you consider) typos. Using the latter makes
for less unnecessary confrontation.
- FChE
|