Philip Martin wrote: > Julian Foad writes: >> Philip Martin <philip.mar...@wandisco.com> >>> vijay <vi...@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. No? (Please sanity-check that because I'm just guessing.) - Julian
