Philip Martin wrote:
> Julian Foad writes:
>> Philip Martin <philip.martin_at_wandisco.com>
>>> vijay <vijay_at_collab.net> writes:
>>>> Attached the patch and log message.
>>> Committed in r1418322. Thanks!
>> Hi Philip and Vijay.
>> Your log message:
>> * subversion/libsvn_ra_serf/replay.c
>> (svn_ra_serf__replay, svn_ra_serf__replay_range): Replace the
>> session url string with the request path portion of the url.
>> leaves me thinking, "But *why* did you replace the session url string
> with the request path portion of the url?"
>> Please edit the log message to say why this change was made and what
>> it means in functional terms (like, what behaviour does it change,
>> does it fix a bug, etc.).
> I don't think a log message is the place for a detailed explanation of
> why a member called 'path' should contain a path rather than a full URL.
I absolutely agree.
> That's the sort of documentation that should go in the code. I suppose
> a log message like: "Set the handler's path member to a path instead of
> a complete URL" might be better.
That would be a little bit better because it gives a hint that it is correcting some sort of error.
But isn't the subject line of this email a relevant clue to what's happening? I don't 100% grok it, but it sounds like the issue being adressed by this patch must be something like:
The mod_dav_svn replay report was logging the entire session
URL in the httpd access log. Make it do what it is supposed
to do, which is log just the path-within-repos.
(Please sanity-check that because I'm just guessing.)
Received on 2012-12-07 16:02:00 CET