Re: [BOOK] [PATCH] Niggles in Chapter 3

Chris Pepper wrote:
>
> Sorry, I read doc/book/HACKING, not /HACKING. I was concerned that

That's OK. Thanks for reading up and re-submitting so soon.

I see that you've now read the bit about attaching a patch to the e-mail; unfortunately you have (or your software has) given it the MIME type "multipart/appledouble" which my mail program has never heard of. If you could attach it as type "text/plain" or "text/x-diff" or "text/x-patch" that would allow me to see it much more easily. If your mail program doesn't give you an option, then it probably chooses a type based on how you saved the file or how you opened the file for attaching, so could you try to save and open it as a simple file of plain text. Sorry for the hassle (yes, I know, things like this are just supposed to work, but ...).

I was able to save the attachment to disk and then read it - it seems to be just plain text inside.

> some of the language nits were subtle enough to require explanation; per
> your suggestion, I've removed those explanations from the below.

It was useful to have the full explanations in your e-mail; it would also be OK to have them in the log message but I felt that it was less necessary to have them there since, by that time, at least two of us will have agreed that they make sense.

> I guessed at the desired level of verbosity for language cleanup
> (not mentioning most of the miscellaneous language tweaks). If more
> detail is required, please let me know, ideally with a sample log
> message illustrating the right level of detail.

That's about right. A little more detail would be OK, and a little less would also be OK.

> Remove spaces around '—' to conform with the style guide. (Note:
> I didn't actually find this spelled out, but Julian Foad confirms the
> preferred style.)

Eek - it's nothing to do with me, gov'nor! I can't find where it says so, either, but it has been stated many times on this e-mail list. (If I commit this patch to the repository then it will implicitly have been approved by me so we won't need this note.)

Comments on the patch itself:

After the phrase "journaled filesystem" you have left an unmatched parenthesis.

"Remember that the 'file:' access method is valid only for locations on the same server as the client" ->
"Remember that the 'file:' access method is valid only for locations on the client"

I'm not sure about this. I agree that the original text sounds silly, but I think your replacement is also odd. "The client" in this context would refer to a Subversion client program. Strictly speaking, this needs to talk about the file system that is (logically) local to the program that is using the 'file:' URL. Note that a rough description has already been given in the table above that paragraph, so it doesn't really need to say anything here. How about just "valid only for local repositories"?

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Oct 3 02:59:48 2004