1314854 - Integrate POD in to REST docs.

Status: RESOLVED FIXED

(Bugzilla :: Documentation, defect)

Description

[Jeff Fearn 🐞]

Attachment: pod2rst.patch

Currently the POD docs for Bugzilla modules, scripts, and Extensions are not integrated in to the REST docs. The POD docs should be integrated in to the docs navigation and theme.

The attached patch converts POD to REST and places it so it gets build into the rest of the REST ;)

Comment 1

[Gervase Markham [:gerv]]

Comment on attachment 8806984 [details] [diff] [review]
pod2rst.patch

Review of attachment 8806984 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Jeff,

Sorry this has taken so long. Thanks for your patience :-)

I think it's a bit confusing to have both an "apis" and an "api" directory, with the first being our Web APIs and the second being Bugzilla's internal APIs. We should rename the second one. "pod" is probably a reasonable name; API seems a bit limiting for what we actually document here.

Quite a bit of our POD seems to have errors the converter complains about. We should fix those in a follow-up patch. (If we put them in this one, there's a risk of bitrot.)

For me, it does 821 files, including every module I have installed locally. That's overkill. I think we only want the Bugzilla modules. You try and avoid doing others, but it doesn't work on my install. My locally-installed modules are in $BUGZILLA_HOME/local/lib/. So this fixed it:

        # Ignoring library files;
        if ($mod =~ /^b?lib::/) {
->
        # Ignoring library files;
        if ($mod =~ /^b?(lib::|local)/) {

This extra stuff won't get built on ReadTheDocs, which doesn't use makedocs.pl. So we'll need a local docs build on bugzilla.org in order to provide it. Will this run into the same problem that we have doing a build of the current system?

There's a FIXME in the patch in conf.py. What's the plan for fixing it? There's definitely a problem with building extension docs - removing the "disabled" file from the Example extension breaks the docs build. You need to find a way of having these two systems (pod2rst and conf.py) not step on each other's toes, so they can run in any order and still work.

In makedocs.pl, you create a "%callbacks" variable and don't use it.

I think it would be better if the index.rst was on the filesystem rather than in a HERE doc. Is there a way of cleaning out the directory without deleting that one file? E.g. temporarily marking it read-only? (Using Git to restore it isn't acceptable, because not all Bugzilla installs are git checkouts.) 

The spacing and indentation doesn't match Bugzilla style - spaces after commas, no space inside function call brackets, etc. Please check.

> open $FH, '<', "$dir_path$out_file" || die "$dir_path$out_file: $!";

Please use File::Spec::Functions for manipulating paths. Did you test this on Windows?

3 random blank lines towards the end of the pod2rst() function.

Comment 2

[Jeff Fearn 🐞]

(In reply to Gervase Markham [:gerv] from comment #1)
> Comment on attachment 8806984 [details] [diff] [review]
> pod2rst.patch
> 
> Review of attachment 8806984 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Jeff,
> 
> Sorry this has taken so long. Thanks for your patience :-)
> 
> I think it's a bit confusing to have both an "apis" and an "api" directory,
> with the first being our Web APIs and the second being Bugzilla's internal
> APIs. We should rename the second one. "pod" is probably a reasonable name;
> API seems a bit limiting for what we actually document here.

There is rst/api/ and rst/integrating/api/, I changed it to rst/integrating/pod/.

> Quite a bit of our POD seems to have errors the converter complains about.
> We should fix those in a follow-up patch. (If we put them in this one,
> there's a risk of bitrot.)

+1, depending on what is causing it we might need to patch pod2rst as well.

> For me, it does 821 files, including every module I have installed locally.
> That's overkill. I think we only want the Bugzilla modules. You try and
> avoid doing others, but it doesn't work on my install. My locally-installed
> modules are in $BUGZILLA_HOME/local/lib/. So this fixed it:
> 
>         # Ignoring library files;
>         if ($mod =~ /^b?lib::/) {
> ->
>         # Ignoring library files;
>         if ($mod =~ /^b?(lib::|local)/) {

Done.

> This extra stuff won't get built on ReadTheDocs, which doesn't use
> makedocs.pl. So we'll need a local docs build on bugzilla.org in order to
> provide it. Will this run into the same problem that we have doing a build
> of the current system?

It shouldn't cause any problems because the rst files aren't there or linked so as far as the other build path is concerned the content just doesn't exist.

> There's a FIXME in the patch in conf.py. What's the plan for fixing it?

Convincing you to drop REST in extensions so we can delete that code path :)

> There's definitely a problem with building extension docs - removing the
> "disabled" file from the Example extension breaks the docs build. You need
> to find a way of having these two systems (pod2rst and conf.py) not step on
> each other's toes, so they can run in any order and still work.

I don't think REST in extensions has much value because it's unlikely people will use REST and POD and that just breaks POD which is *terrible* for a perl module to do.

> In makedocs.pl, you create a "%callbacks" variable and don't use it.

It's used as a parameter to Pod::POM::View::Restructured->new.

> I think it would be better if the index.rst was on the filesystem rather
> than in a HERE doc. Is there a way of cleaning out the directory without
> deleting that one file? E.g. temporarily marking it read-only? (Using Git to
> restore it isn't acceptable, because not all Bugzilla installs are git
> checkouts.) 

It isn't just the top here doc, it updates the file for every pod file it finds.

Two issues I ran in to when doing it that way were:

1: If you build the docs without using makedocs.pl it wasn't happy because the linked files where missing.

2: Every time you enable/disable/install/remove an extension you have to update the file, this way it automatically updates.

> The spacing and indentation doesn't match Bugzilla style - spaces after
> commas, no space inside function call brackets, etc. Please check.

My time is too valuable too be spent on something that was automated 16 years ago, your time is also too valuable to be spent on such things. If someone supplies a perltidy rc file I am more than happy to use it.

> > open $FH, '<', "$dir_path$out_file" || die "$dir_path$out_file: $!";
> 
> Please use File::Spec::Functions for manipulating paths.

Cool, this will take a little longer to do.

> Did you test this on Windows?

Nah, I don't have a test machine at work for Windows. I can take a shot at testing over the holiday break though.

> 3 random blank lines towards the end of the pod2rst() function.

I'll blame Matt for those :D

Cheers, Jeff.

Comment 3

[Jeff Fearn 🐞]

(In reply to Jeff Fearn from comment #2)
> (In reply to Gervase Markham [:gerv] from comment #1)
> > I think it's a bit confusing to have both an "apis" and an "api" directory,
> > with the first being our Web APIs and the second being Bugzilla's internal
> > APIs. We should rename the second one. "pod" is probably a reasonable name;
> > API seems a bit limiting for what we actually document here.
> 
> There is rst/api/ and rst/integrating/api/, I changed it to
> rst/integrating/pod/.

Just to confirm, this is existing behavior, and you want me to change it along with base_api_url, right?

Comment 4

[Gervase Markham [:gerv]]

(In reply to Jeff Fearn from comment #3)
> Just to confirm, this is existing behavior, and you want me to change it
> along with base_api_url, right?

Yes, I think so. I want there to not be two directories called "api", one with the Web API, the other with the internal API. "pod" is not a great name either, as that's an implementation detail. How about "internals"?

Gerv

Comment 5

[Jeff Fearn 🐞]

Attachment: 1314854.patch

Think I got everything, included running perl tidy on file as it's almost completely changed anyway.

Comment 6

[Gervase Markham [:gerv]]

Comment on attachment 8841433 [details] [diff] [review]
1314854.patch

r=gerv; Sorry it's taken so long.

Gerv

Comment 7

[Gervase Markham [:gerv]]

To [e-mail]:bugzilla/bugzilla.git
   8dae231..bae90f4  master -> master

(With a few formatting tweaks to restore the file to the standard Bugzilla style.)

Thanks, Jeff!

Gerv