More codebase cleanup, perlcritic, and POD coverage

TIme to take a look at all the other stuff lying around in the codebase.

  • INSTALL is pretty tied up with the details of hooking a non mod_perl CGI script into Apache. Some of it will be salvageable, but mostly not. The section on first-run may be useful, but definitely there’s going to need to be a way to set up a privileged account before the first run.
  • LICENSE will need to be updated with the original release date. I’ll go look at the ibiblio site for that.
  • Making a note to move THANKS into README.
  • USING is mostly okay. It’s a very quick intro to using a wiki. The section on logging in will need some editing, as it assumes the WikiWiki model of “anyone can add an account”.
  • The bin/insert-mail script was a hack specifically for our use as a bug tracker. We probably don’t need it, and there are significant security issues to address if we decide we do. Deleting this; we can always get it out of Git if we change our minds.
  • The cgi-bin directory can go away; the script there really just calls the code we moved to App::WebWebXNG.pm.
  • The docs directory contains a set of fixed HTML documents. They probably want a reformatting and possibly a rewriting, but we can leave them as they are right now.
  • Everything in old-lib has been moved elsewhere; that can go away.

Back to the code!

I revamped the dist.ini file to use [@Basic] and removed the duplicate actions. It now looks like this:

[AutoPrereqs]
[@Basic]
[PruneCruft]
[ExtraTests]
[Test::Perl::Critic]
[PodCoverageTests]
[PodSyntaxTests]
[@Git]

The next step is to get everything tidied up and passing the perlcritic tests. To that end, I moved the start of main() in App::WebWebXNG to only encompass the actual old main program and added a leading underscore to _setup_kludge. That keeps us from having to document something we’ll be removing anyway, and un-nests the rest of the methods in that module to get rid of a huge number of perlcritic errors.

I’ve also moved the old PasswordManager code to App::WebWebX::AuthManager; the old code manages an Apache htpasswd basic auth file, but the structure will do as an interface to some kind of more modern authentication management. (Notable in there: no password requirements! It was a simpler time.)

Next is to remove the code we definitely don’t need or want: the insert-mail script, the CGI wrapper, everything in old-lib, and the license file from GitHub (Dist::Zilla will generate one).

>File::LockDir fiddles with the symbol table, and I don’t want to do that any more. I’ll restructure it as a class. It’ll also need some tests, and I’ll have to start writing those and fixing up the code to pass.

The perlcritic tests and POD coverage tests are running, and failing, so I’ll need to start fixing those. I started on this and actually realized that I hadn’t committed the tidied code before starting to work on it, so I created a branch, wound the history back with git reset, committed the tidied code, and then cherry-picked back to the current state. This let me keep the critic and POD changes separate.

For the modules failing the POD tests, I’d actually added block comments that would work perfectly fine as POD when I originally wrote the code, so I just needed to do the mechanical task of converting them. There were a lot of them, but it was very easy editing, so I just went ahead and cleaned that up by hand.

Critic fixes were primarily making all of the loop variables lexical and removing bare word file handles. There are two methods that store and reload the global config that use a string eval(); they’re ## no critic marked for now, but I want to think about a better setup for that. My current reflexes say “database”, but I’m trying to minimize dependencies. Let’s see how that goes and defer the decision.

I started adding some tests: if PageArchive gets no directory, it should die.

At this point, we pass all of the tests that we have; the code is barely tested, but the POD and critic tests, which had a lot of errors, are all fixed, and the couple of validation tests I added are passing.

That will do for this pass.

Reply