Re: [PATCH] code file names linkified in general.html of the Hacking Guide

[Ben Reser <be...@reser.org>]

On Wed, Dec 26, 2012 at 4:27 PM, Gabriela Gibson
<ga...@gmail.com> wrote:
> * docs/community-guide/general.part.html
>
>   (code-to-read): Linkified code file names.

You've got an extra blank line in there between the file and the identifier.

> * docs/community-guide/general.part.html
>   (directory-layout): Linkified code file names.

You can list multiple identifiers in the same line comma separated if
they have the same description.

Here's an example of that type of collapsing:
http://svn.apache.org/r1428209

> @@ -218,72 +281,132 @@ to ask for review on the developer mailing list.</
>
>  <div class="h2" id="directory-layout">
>  <h2>Directory layout
> -  <a class="sectionlink" href="<!--#echo var="GUIDE_GENERAL_PAGE" -->#directory-layout"
> +  <a class="sectionlink" href="general.html#directory-layout"
>      title="Link to this section">&para;</a>
> </h2>
>
> <p>A rough guide to the source tree:</p>
>
> <ul>

You've removed the SSI code that generates the page in the above diff
hunk.  You might want to work off a checkout from:
https://svn.apache.org/repos/asf/subversion/site

> -<li><p><tt>tools/</tt><br />
> +
> +<li><p><tt><a>
> +    href="https://svn.apache.org/repos/asf/subversion/trunk/tools/">
> +	tools/</a></tt><br />
> +

Broken HTML.

>  </li>
> +
>  <li><p><tt>packages/</tt><br />

Missed one/spurious whitespace addition.

> -<li><p><tt>neon/</tt><br />
> +
> +<li><p><tt><a
> +    href="http://www.webdav.org/neon/doc/html">neon/</a></tt><br />
> +

Neon is no longer used by SVN in trunk, so I'd remove Neon.  Serf has
replaced neon but the intent herein is to describe the layout of the
source tree.  We don't support in-tree building of Serf.  So I
wouldn't bother to replace it.

We do still support in-tree building of APR but frankly, I'm not sure
this is really important to have here anymore.  I think the other
references are enough.

With respect to Bert's comments I generally agree, I'd link to the
documentation files which have links to the source code a
pretty-printed HTML copy of the source code.  E.G. svn_client.h would
be:
http://subversion.apache.org/docs/api/latest/svn__client_8h.html

Re: [Patch] Re: [PATCH] code file names linkified in general.html of the Hacking Guide

[Stefan Sperling <st...@apache.org>]

On Sun, Jan 06, 2013 at 09:38:45PM +0000, Gabriela Gibson wrote:
> On 04/01/13 04:07, Ben Reser wrote:
> (snip)
> 
> Thank you very much for the comments.
> 
> A new patch is attached.

Thanks, committed in r1430275.

Two minor nits which I left unfixed:

- You didn't linkify the packages/ directory of the source tree.
- The log message didn't explain *why* neon and apr were removed
  from the listing. (We could still edit the log message.)

If you want to fix any of these, feel free to do so.
But it's not strictly necessary from my point of view.

Re: [PATCH] Re: [Patch] Re: [PATCH] code file names linkified in general.html of the Hacking Guide

[Ben Reser <be...@reser.org>]

On Tue, Jan 8, 2013 at 8:36 AM, Ben Reser <be...@reser.org> wrote:
> Not sure how I managed to leave the list off.  If that happens again
> feel free to add the list back on.
>
> Looks like Stefan has committed an earlier version of your patch.
> I'll go ahead and fix up the changes from this patch to apply over
> what he committed.

Done in r1430384.  Outside of rebasing the patch the only thing I did
was remove a duplicate trailing slash on the packages link.

Thanks for the contribution.

Re: [PATCH] Re: [Patch] Re: [PATCH] code file names linkified in general.html of the Hacking Guide

[Ben Reser <be...@reser.org>]

Not sure how I managed to leave the list off.  If that happens again
feel free to add the list back on.

Looks like Stefan has committed an earlier version of your patch.
I'll go ahead and fix up the changes from this patch to apply over
what he committed.

On Mon, Jan 7, 2013 at 12:53 PM, Gabriela Gibson
<ga...@gmail.com> wrote:
> On 07/01/13 01:15, Ben Reser wrote:
>>
>> On Sun, Jan 6, 2013 at 1:38 PM, Gabriela Gibson
>> <ga...@gmail.com> wrote:
>>>
>>> A new patch is attached.
>>
>>
>> Getting closer.
>>
>>>   (code-to-read): Create links for all file names to point to HTML
>>>    pretty-print versions.
>>
>>
>> I'd say you're linking to the docs, the pretty print version is linked
>> off the page you're linking to.  If there weren't other things I'd
>> have just changed this and committed it.
>>
> Hi Ben,
>
> Many thanks, your patience is very much valued.
>
> I don't like to chatter too much on the dev list to keep the bandwidth low,
> but I have a blog now that OPW asked me to start, and so I thought I share
> my patch woes with the other interns here:
>
> http://gabriela-gibson.blogspot.co.uk/2013/01/how-to-mess-up-patch-part-i.html
>
> Please keep being picky, otherwise, I won't ever learn :)
>
> regards
>
> Gabriela
> Ps.: you mailed me directly, I was not sure what to do in that case, as this
> actually should be on the mailing list, but I added chatter that I normally
> would not in this case.  Should I repost the 'business version' of this on
> the dev list?
>
>
>

[Patch] Re: [PATCH] code file names linkified in general.html of the Hacking Guide

[Gabriela Gibson <ga...@gmail.com>]

On 04/01/13 04:07, Ben Reser wrote:
(snip)

Thank you very much for the comments.

A new patch is attached.