283231 - POD for Bugzilla::FlagType

Status: RESOLVED FIXED

(Bugzilla :: Documentation, enhancement)

Description

[Kevin Benton]

POD for Bugzilla::FlagType

Comment 1

[Kevin Benton]

Attachment: POD for Bugzilla::FlagType

Comment 2

[Kevin Benton]

Comment on attachment 175222 [details] [diff] [review]
POD for Bugzilla::FlagType

Sorry guys - wrong file.

Comment 3

[Kevin Benton]

Attachment: POD for Bugzilla::FlagType

Comment 4

[Max Kanat-Alexander]

Comment on attachment 175222 [details] [diff] [review]
POD for Bugzilla::FlagType

>+FlagType.pm provides an interface to flag types as stored in Bugzilla.

  Bugzilla::FlagType.

>+=item *
>+
>+Prior to calling routines in this module, it's assumed that you have
>+already done a C<require CGI.pl>.

  This eventually will change in a future version, when CGI.pl is removed. You
might want to note that.

>+=item *
>+
>+Use of private functions/variables outside this module may lead to
>+unexpected results after an upgrade.  Please avoid using private
>+functions in other files/modules.

  Explain that private functions are ones that start with _ or are specifically
noted as being private, so that people who aren't core BZ devs but want to make
modifications know about that.

>+=back
>+
>+=head1 LICENSE

  I'd prefer that the license exist only in one place, and that place be at the
top of the file, in normal code comments.

  It looks like there aren't any actual subroutine docs in here... are they
going to be added later?

Comment 5

[Max Kanat-Alexander]

Comment on attachment 175226 [details] [diff] [review]
POD for Bugzilla::FlagType


  Ahh, that's a bit better. :-) My comments from above still stand, in addition
to any comments here:

>-# basic sets of columns and tables for getting flag types from the database
>+=head1 PRIVATE VARIABLES/CONSTANTS

  This should be surrounded by a =begin undocumented and =end undocumented. The
private stuff should not be exposed in public POD.

>+use constant @base_columns = 

  You can't change things to use constant without actual code modification
elsewhere. This won't work. Just leave it as it is now, and we'll change that
in another patch.


  OK, I'm just going to comment on the POD generally:

  (1) I'm thrilled to have this. Any POD is better than no POD. :-)
  (2) The Bugzilla::DB POD docs have good style for the subroutine
documentation, with the "Description, Params, Returns" sections -- let's follow
that for all the POD that we can. If you don't want to do that, I'll totally
understand, but if you can do it it would help out a lot. :-)
  (3) We need a SYNOPSIS section that shows how to use all the functions.

  Also, the *best* POD that I've seen also has the little "example" sections at
the top of each subroutine POD (see the DBI docs). That's not totally
necessary, but if you want to do it, it would be awesome. :-)

Comment 6

[Kevin Benton]

Attachment: Changes as requested by Max

Comment 7

[Max Kanat-Alexander]

Comment on attachment 175357 [details] [diff] [review]
Changes as requested by Max

Looks good to me. My comments from the Flag review also apply.

Oh, and did you leave the CONTRIBUTORS section in there on purpose?

Comment 8

[Max Kanat-Alexander]

Hey Kevin -- there's a bit of bitrot that I don't feel comfortable fixing on
checkin. Could you post a new patch?

Comment 9

[Kevin Benton]

Attachment: Updates to fix bitrot

Re-applied same patch against updated code.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 177507 [details] [diff] [review]
Updates to fix bitrot

bootyful

Comment 11

[Max Kanat-Alexander]

Checking in Bugzilla/FlagType.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/FlagType.pm,v  <--  FlagType.pm
new revision: 1.12; previous revision: 1.11
done