Web lists-archives.com

Re: [PATCH 2/2] trace2: randomize/timestamp trace2 targets




Jeff King <peff@xxxxxxxx> writes:

> This definitely seems useful. Could we drop the final "%" and make it
> either a single-character "%t" or "%(iso8601)" to match our other
> formatting strings? There's no _technical_ reason to do that, but it
> just seems like there's not much point in being unnecessarily
> inconsistent.

I do agree that %TOKEN% is not among the patterns we've used before,
and I fully anticipated that it would attract paintbrushes when the
proposal hits the public list ;-)

A big question is what we want to be consisten with, though.  From
the readability point-of-view, for-each-ref language %(token) is
easier to read and extend.  A secondary question is that there
likely are things other than the timestamp of the time process
started that may want to be interpolated, so we may want to pick
some useful vocabulary upfront.  If we can declare interpolation
will be done ONLY for timestamps, as Ævar suggests, taking strftime
pattern directly from the caller may be sufficiently simple to
explain and useful, but I am not sure "only for timestamps" is a
limitation we would want to adopt this early in the design process.

> Doing this automatically for directories feels kind of magical. I'd have
> expected it to be just another placeholder. And in fact, I'd think using
> the process id as a placeholder would be pretty common. Between
> timestamp and pid, that's generally unique (though I'm not opposed to
> the random placeholder if somebody really wants to be thorough).
>
> That leaves the door open for being able to append to or overwrite
> existing trace files later, if we want to make that a possibility.

Thanks.