.\" Automatically generated by Pod::Man 2.28 (Pod::Simple 3.29) .\" .\" Standard preamble: .\" ======================================================================== .de Sp \" Vertical space (when we can't use .PP) .if t .sp .5v .if n .sp .. .de Vb \" Begin verbatim text .ft CW .nf .ne \\$1 .. .de Ve \" End verbatim text .ft R .fi .. .\" Set up some character translations and predefined strings. \*(-- will .\" give an unbreakable dash, \*(PI will give pi, \*(L" will give a left .\" double quote, and \*(R" will give a right double quote. \*(C+ will .\" give a nicer C++. Capital omega is used to do unbreakable dashes and .\" therefore won't be available. \*(C` and \*(C' expand to `' in nroff, .\" nothing in troff, for use with C<>. .tr \(*W- .ds C+ C\v'-.1v'\h'-1p'\s-2+\h'-1p'+\s0\v'.1v'\h'-1p' .ie n \{\ . ds -- \(*W- . ds PI pi . if (\n(.H=4u)&(1m=24u) .ds -- \(*W\h'-12u'\(*W\h'-12u'-\" diablo 10 pitch . if (\n(.H=4u)&(1m=20u) .ds -- \(*W\h'-12u'\(*W\h'-8u'-\" diablo 12 pitch . ds L" "" . ds R" "" . ds C` "" . ds C' "" 'br\} .el\{\ . ds -- \|\(em\| . ds PI \(*p . ds L" `` . ds R" '' . ds C` . ds C' 'br\} .\" .\" Escape single quotes in literal strings from groff's Unicode transform. .ie \n(.g .ds Aq \(aq .el .ds Aq ' .\" .\" If the F register is turned on, we'll generate index entries on stderr for .\" titles (.TH), headers (.SH), subsections (.SS), items (.Ip), and index .\" entries marked with X<> in POD. Of course, you'll have to process the .\" output yourself in some meaningful fashion. .\" .\" Avoid warning from groff about undefined register 'F'. .de IX .. .nr rF 0 .if \n(.g .if rF .nr rF 1 .if (\n(rF:(\n(.g==0)) \{ . if \nF \{ . de IX . tm Index:\\$1\t\\n%\t"\\$2" .. . if !\nF==2 \{ . nr % 0 . nr F 2 . \} . \} .\} .rr rF .\" ======================================================================== .\" .IX Title "Net::Citadel::ToDo 3pm" .TH Net::Citadel::ToDo 3pm "2016-08-31" "perl v5.22.2" "User Contributed Perl Documentation" .\" For nroff, turn off justification. Always turn off hyphenation; it makes .\" way too many mistakes in technical documents. .if n .ad l .nh .SH "NAME" Net::Citadel::ToDo \- To Do items for the Net::Citadel Perl extension. .SH "TODO" .IX Header "TODO" .SS "General" .IX Subsection "General" 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 \f(CW\*(C`gitpan\*(C'\fR \&\s-1CPAN\s0 mirror at GitHub? .PP Add capability to use \s-1SSL\s0 encrypted communications with the Citadel server, perhaps using IO::Socket::SSL? .PP Change to using the jame@cpan.org address for Robert James Clay instead of the jame@rocasa.us address? .SS "Constants" .IX Subsection "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. .PP A \*(L"000\*(R" string constant is used quite often by Citadel but is not defined in the module. .PP There is a set of follow up reply codes noted in the documentation that are not yet in the module. .PP Add a link to the online documentation for the status codes: http://www.citadel.org/doku.php?id=documentation:appproto:statuscodes .PP A \f(CW\*(C`NO_SUCH_USER\*(C'\fR return code is mentioned in the command documentation (for the \s-1USER\s0 command, for instance,) but the module does not currently appear to define it. .PP Add a paragraph to the documentation after the \s-1CONSTANTS\s0 header. .PP Add a paragraph to the documentation after the 'Results Code' header, perhaps including a link to the online status codes page in Citadel Documentation. .PP 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. .SS "Functions" .IX Subsection "Functions" There does not seem to an explicit function to close the connection; i.e., close the socket being used to communicate with the server. .PP Create an internal function to handle \s-1LISTING_FOLLOWS\s0 return lines from the server? Perhaps called something like \f(CW\*(C`_get_listing\*(C'\fR? .ie n .IP """assert_room"" function" 4 .el .IP "\f(CWassert_room\fR function" 4 .IX Item "assert_room function" Is the \f(CW\*(C`assert_room\*(C'\fR command coming up with the correct floor information? .Sp Is the \f(CW\*(C`assert_room\*(C'\fR function sending the parameters for the \s-1CRE8\s0 command properly? (There is a note in the code indicating an uncertainty about what should be sent.) .ie n .IP """retract_room"" function" 4 .el .IP "\f(CWretract_room\fR function" 4 .IX Item "retract_room function" In the existing test code for the \f(CW\*(C`retract_floor\*(C'\fR function, there is a comment saying \f(CW\*(C`CITADEL BUG\*(C'\fR; 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. .ie n .IP "function ""new""" 4 .el .IP "function \f(CWnew\fR" 4 .IX Item "function new" The line just before the 'return \f(CW$self\fR' line actually has two commands on it, not just one. The second one is just \*(L"<$s>;\*(R" and that has the comment \&\*(L"# consume banner\*(R". That 'banner' is something like the following: \*(L"200 Citadel server ready.\*(R" Instead of not doing anything with that, save it somewhere? At least the part of it? (The first command on that line just establishes the \f(CW$s\fR variable for that read.) At least, put them on separate lines. .ie n .IP """citadel_time"" function" 4 .el .IP "\f(CWcitadel_time\fR function" 4 .IX Item "citadel_time function" The current \f(CW\*(C`citadel_time\*(C'\fR function only returns the first two parameters from the \s-1TIME\s0 command: \f(CW\*(C`1347624956|\-14400\*(C'\fR. The Citadel \s-1TIME\s0 command itself actually returns a line like this: \f(CW\*(C`200 1347625545|\-14400|1|1347537300\*(C'\fR, with the '200' being the \s-1OK\s0 code and the rest being the four fields that it returns (which also needs to be documented). So as currently written, the function does not 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. .Sp Rewrite the \f(CW\*(C`citadel_time\*(C'\fR function to unpack the parameters that the \&\s-1TIME\s0 command returns when it is successful, and then return them to the calling program in a hash? Could also then return the hash with a key named 'error' or 'Error' if it is not successful. Update its \s-1POD\s0 to reflect the changes. .ie n .IP """citadel_echo"" function" 4 .el .IP "\f(CWcitadel_echo\fR function" 4 .IX Item "citadel_echo function" It appears to test if what was sent, was actually received back but it only returns true if it does not fail because there is no match. Add an explicit return of the value returned from the \s-1ECHO\s0 command? .SS "Documentation" .IX Subsection "Documentation" Add something like Net::Citadel::Tutorial for examples of the use and configuration of the module? .SS "Testing" .IX Subsection "Testing" Will the differences with the newer Citadels justify or neccessitate checking the version of the Citadel being tested and doing different tests depending on that version? .PP Change the names of the test floors and rooms to something like \f(CW\*(C`Test Floor\*(C'\fR and \f(CW\*(C`Test Room\*(C'\fR? .PP Update the testing for the \f(CW\*(C`citadel_time\*(C'\fR function to at least check the number of parameters returned? Three of the parameters are Unix timestamps; 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'. .PP When testing for a floor, it looks for \f(CW\*(C`Main Floor\*(C'\fR: that exists on a default install but may not be on an working system. Make it another configuration item, which if not configured would default to \f(CW\*(C`Main Floor\*(C'\fR? .PP Separate the testing to: functions that do not require a log in, those that do 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? .PP Since Config::YAML is already a build-depends and is used by the main test script, add a \f(CW\*(C`Testing\*(C'\fR 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? .PP Instead of having the debugging 'warning' lines commented out, use a \s-1DEBUG\s0 environment variable and/or a test.yaml configuration item for it. Same for the 'use Data::Dumper' line itself. .PP Use Config::YAML::Tiny instead of Config::YAML? (Does not appear to be in Debian as yet, although YAML::Tiny is.) .PP Use something like Test::MockObject to do at least basic testing of the various functions. And/or Test::TCP? .PP Testing of the citadel_info function currently just checks the number of information lines returned by the server. That could change over time and different Citadel versions. Make that a testing configuration item? .PP Do more extensive checking of what is returned by the citadel_info function? .SH "SEE ALSO" .IX Header "SEE ALSO" .Vb 1 \& L .Ve .SH "AUTHOR" .IX Header "AUTHOR" Robert James Clay, \f(CW\*(C`\*(C'\fR .SH "COPYRIGHT AND LICENSE" .IX Header "COPYRIGHT AND LICENSE" Copyright 2016 Robert James Clay, all rights reserved. .PP This program is free software; you can redistribute it and/or modify it under the same terms as Perl itself.