[SCM] Packaging of Net::Citadel in Debian branch, debian, updated. debian/0.16-1-45-gd21e85b

[Robert James Clay]

The following commit has been merged in the debian branch:
commit f2ab228f571e5391991e6cec794b101e833733f3
Author: Robert James Clay <jame at rocasa.us>
Date:   Tue Oct 16 15:24:59 2012 -0400

    Net::Citadel ToDo as of 16 October 2012.

diff --git a/lib/Net/Citadel/ToDo.pod b/lib/Net/Citadel/ToDo.pod
index bb8358f..025f70f 100644
--- a/lib/Net/Citadel/ToDo.pod
+++ b/lib/Net/Citadel/ToDo.pod
@@ -9,47 +9,50 @@ Net::Citadel::ToDo - To Do items for the Net::Citadel Perl extension.
 
 =head2 General
 
-Add a function for the MRTG command, possibly as a separate module like
-C<Net::Citadel::MRTG> since it should be in the C<Net::Citadel> namespace
-but does not require a login and therefore does not need the C<new> 
-function in the main module.
+Add a function for the MRTG command, possibly as the function C<citadel_mrtg>.
 
 Add a function for the INFO command, possibly as the function C<citadel_info>.
 
 Add a ZIPFLAGS configuration item for dist/zipdist in Makefile.PL?
 
-Add a note in the documenation about the Net::Citadel repository and home page
-at GitHub? Also include a refernce to where it is available at the C<gitpan>
+Add a note in the documentation about the Net::Citadel repository and home page
+at GitHub? Also include a reference to where it is available at the C<gitpan>
 CPAN mirror at GitHub?
 
 
-=head2 Contstants
-
-Use the Readonly module instead of C<use constant> for the constants being 
-defined in the module.
+=head2 Constants
 
 There are some numbers being used that are actually constants; change to
 using Readonly to define them for use in the code, unless there are other
 standard constants that can be used, like those for the flock operations.
 
-When converting the user related constants, also move them up with the
-rest of the constants definitions.
+Add a paragraph to the documentation after the CONSTANTS header.
+
+Add a paragraph to the documentation after the 'Results Code' header, perhaps
+including a link to the online status codes page in Citadel Documentation.
+
+Add a paragraph to the documentation after the 'Room Access' header, perhaps
+including a link to the online page in Citadel Documentation that refers to
+them.
+
+A "000" string constant is used quite often by Citadel but is not defined in the
+module.
 
-Add POD documentation for the constants, inline with where they are defined
-so it also serves as documentation when looking at the source.
+There is a set of follow up reply codes noted in the documentation that are not
+yet in the module.
 
 A C<NO_SUCH_USER> return code is mentioned in the command documentation (for
 the USER command, for instance,) but the module doesn't currently appear to
 define it.
 
+Add a link to the online documentation for the status codes:
+http://www.citadel.org/doku.php?id=documentation:appproto:statuscodes
 
-=head2 Functions
 
-Add an explicit C<return> line as necessary to those subroutines that do not
-have them.
+=head2 Functions
 
-Export some functions, at least using C<EXPORT_OK>? Perhaps for the ones that do
-require a login to run?
+There does not seem to an explicit function to close the connection; i.e., close
+the socket being used to communicate with the server.
 
 =over 4
 
@@ -62,10 +65,24 @@ properly? (There is a note in the code indicating an uncertainty about what
 should be sent.)
 
 
+=item C<retract_room> function
+
+In the existing test code for the C<retract_floor> function, there is a comment
+saying C<CITADEL BUG>; investigate if it an issue with the function itself, an
+actual issue with Citadel, and/or just something to do with the test script.
+The documentation for the function itself refers back to Citadel v7.20, so there
+may not be any issue with it now.
+
+
 =item function C<new>
 
-Add being able to set the port option; if not present, it would default to
-the standard Citadel port number (using the C<CITADEL_PORT> constant).
+The line just before the 'return $self' line actually has two commands on it,
+not just one. The second one is just "<$s>;" and that has the comment
+"# consume banner".  That 'banner' is something like the following:
+ "200 <systen name> Citadel server ready."  Instead of not doing anything
+with that, save it somewhere? At least the <system name> part of it?  (The
+first command on that line just establishes the $s variable for that read.)
+At least, put them on separate lines.
 
 
 =item C<citadel_time> function
@@ -74,9 +91,10 @@ The current C<citadel_time> function only returns the first two parameters
 from the TIME command:  C<1347624956|-14400>.  The Citadel TIME command itself
 actually returns a line like this: C<200 1347625545|-14400|1|1347537300>,
 with the '200' being the OK code and the rest being the four fields that it
-returns. So as currently written, the function doesn't return the daylight
-savings time indication and the actual citadel server start information.
-Function first needs to be changed to at least return all parameters.
+returns (which also needs to be documented). So as currently written, the
+function doesn't return the daylight savings time indication and the actual
+citadel server start information. Function first needs to be changed to at
+least return all parameters.
 
 Rewrite the C<citadel_time> function to unpack the parameters that the
 TIME command returns when it is successful, and then return them to
@@ -87,10 +105,9 @@ to reflect the changes.
 
 =item C<citadel_echo> function
 
-It appears to test if what was sent was actually received back but doesn't
-explicitly return any indication one way or another; the function just
-croaks if there is no match. Add an explicit return of the value
-returned from the command? Or just return true for a normal exit?
+It appears to test if what was sent was actually received back but only returns
+true if it doesn't fail because there is no match. Add an explicit return of
+the value returned from the ECHO command? 
 
 =back
 
@@ -108,11 +125,9 @@ and C<Test Room>?
 
 In the testing related to users, change to using the name TestUser.
 
-Implement the testing for the C<retract_floor> function.
-
 Update the testing for the C<citadel_time> function to at least check the
 number of parameters returned? Three of the parameters are Unix timestamps;
-validate those in some way? The fourth parameter being returned is a Boolean
+validate those in some way? The other parameter being returned is a Boolean
 and is used to indicate Daylight Savings Time and should be a '0' or a '1'.
 
 When testing for a floor, it looks for C<Main Floor>; that exists on
@@ -124,7 +139,20 @@ require a log in, those that are read only, those that write to the server. Or
 use the separation given in the documentation for the different Sections for
 the commands?
 
-Add a C<Testing> section of some sort to the configuration file?
+Since Config::YAML is already a build-depends and is used by the main test
+script, add a C<Testing> section of some sort to the test.yaml file and check
+that for the various options for testing instead of just attempting to reach a
+Citadel server?
+
+Instead of having the debugging 'warning' lines commented out, use a DEBUG
+environment variable and/or a test.yaml configuration item for it. Same
+for the 'use Data::Dumper' line itself.
+
+Use Config::YAML::Tiny instead of Config::YAML? (Does not appear to be in
+Debian as yet, although YAML::Tiny is.)
+
+Use something like Test::MockObject to do at least basic testing of the 
+various functions.  And/or Test::TCP?
 
 
 =head1 SEE ALSO

-- 
Packaging of Net::Citadel in Debian