328601 - Add POD to Bug.pm

Status: NEW

(Bugzilla :: Bugzilla-General, defect, P1)

Description

[Frédéric Buclin]

.

Comment 1

[Sam Folk-Williams]

I'll work on this.

Comment 2

[Sam Folk-Williams]

Frederic - a couple of questions as I work through this... looks like there are over 100 functions. In looking at other files, it seems the POD doesn't necessarily document every singe function. Is there some kind of guideline as to what needs to be documented? Or should I just shoot for all of them? (eventually..)

Also, wondering if you could write the SYNOPSIS?

-Sam

Comment 3

[Frédéric Buclin]

Only public methods and subroutines should be described. Those beginning with an underscore (e.g. _check_product()) should not appear in the POD.

Comment 4

[Sam Folk-Williams]

Attachment: outline of pod

This is an outline plus one function described - I want to have this reviewed before I go further to see how far off the mark i am.

btw... have been on vacation and tied up with year end stuff, but should be back online now.

-Sam

Comment 5

[Max Kanat-Alexander]

Comment on attachment 294699 [details] [diff] [review]
outline of pod

You don't need to re-describe the constants and methods from Bugzilla::Object, except what you did for create() is good because that's all the Bugzilla::Bug-specific stuff.

Each param should be an =item, so that means that Params should have an =over under it and a =back after its items.

Look at Bugzilla::DB for an example of what method POD is supposed to look like.

Comment 6

[Max Kanat-Alexander]

Note that for a lot of the subs, they've already been documented in bug 329301 and you can steal the POD from there.

Comment 7

[Sam Folk-Williams]

Just want to clarify this - what I'm getting from looking at other files would indicate this form for a function:

-------------
=item C<bug_alias_to_id($alias)>

 Description: Converts a bug alias into a numeric bug id.

 Params:      $alias - The alias of the bug, a string.

 Returns:     The numeric bug id if the alias exists, or undef if there
              is no bug with that alias.

=over
-------------------

But, in some it seems that "Description", "Params" and "Returns" are all "=item B" items and thus have an =over tag after them. But it doesn't seem consistent. I think the above would be:
------------------
=item C<bug_alias_to_id($alias)>

=over

=item B<Description> 

Converts a bug alias into a numeric bug id.

=over

=item B<Params>      

$alias - The alias of the bug, a string.

=over

=item B< Returns>     

The numeric bug id if the alias exists, or undef if there is no bug with that alias.

=over

=back
----------

Which is correct, if either?

Comment 8

[Frédéric Buclin]

Personally, I prefer the first syntax; it's more compact. But I know mkanat prefers the second one. I also know mkanat and I disagree on this. ;)

Comment 9

[Sam Folk-Williams]

Well, I think I'll stick with the first one as i agree it's more compact.

mkanat - is that OK?

-Sam

Comment 10

[Max Kanat-Alexander]

I think that most of you are used to reading POD as raw code, instead of its actual output format. But anybody who's used to reading it as raw code doesn't even need to be reading it, since they're reading the code of the method itself.

Trust me, the second form is much more readable in the actual output format.

Also, the parameters should not be specified in the name, because that makes it impossible to link to the method from other POD or within the POD itself.

Finally, Params actually needs to look like this:

=item B<Params>

=over

=item C<$alias> - Some description here

=back

Comment 11

[Max Kanat-Alexander]

The first syntax also formats to preformatted text, which looks pretty bad in "perldoc" format, and doesn't look much better in HTML format.

Anybody writing or reviewing POD really should read the http://perldoc.perl.org/perlpod.html documentation page.

Comment 12

[Max Kanat-Alexander]

For example, here's some bad output:

http://www.bugzilla.org/docs/tip/html/api/Bugzilla/DB/Schema.html#new
http://www.bugzilla.org/docs/tip/html/api/importxml.html#DESCRIPTION

And here's some good output:

http://www.bugzilla.org/docs/tip/html/api/Bugzilla/DB.html#bz_last_key

Comment 13

[Max Kanat-Alexander]

Oh, and your second format is actually incorrect. Please do read the POD of Bugzilla::DB for the correct formatting.

It should look like this:

=head2 Some Heading

=over

=item C<some_method_name>

=over

=item B<Description>

Description goes here

=item B<Params>

=over

=item C<$param1> - Some param.

=item C<$param2> - Some other param.

=back

=item B<Returns>

A brief description of what type and thing this returns.

=back

=item C<some_other_method>

=over

(repeat the Description, Params, Returns bit here)

=back

=back

(That last =back closes the =over that started under the head.)

Comment 14

[Sam Folk-Williams]

re-assigning this for now - given my lack of pod/perl fu i think someone else might be able to tackle this more easily. i'll focus on other bugs.

Comment 15

[Frédéric Buclin]

Attachment: template to use for each method

This is the template which should be used for each single method. As there are tons of methods in Bugzilla::Bug, I suggest we simply attach this template for each method (or a small set of methods) as text/plain, not as patches. This way, we avoid conflicts (as they are all independent), and we can review POD for each method (or small set of methods) individually. When there is POD for all methods, it's easy to combine them all in a single file as it's a simple concatenation of all attachments (+ sorting them alphabetically, at least within each logical bloc). It can then be committed to CVS.

Comment 16

[bummya51507]

Dev tools for bug threats