Comment 5 for bug 1947898

These are my "first impression" comments from having looked at the code and the documentation, but not having run it, yet. These are intended as constructive/actionable suggestions to make it fit better with the existing Evergreen code base and way of doing things. These are my opinions, so take them for what they're worth. Nothing here is meant to be negative.

This code adds the following dependencies to Evergreen:

* DateTime::Format::Duration
* Digest::SHA1
* Email::Sender::Simple
* pQuery

Of the above, the functionality of Digest::SHA1 can be replaced by the sha1 functions of Digest::SHA, which this new code also uses. I suggest refactoring the code to use Digest::SHA and dropping the Digest::SHA1 dependency.

Evergreen currently uses Email::Send to deliver email. Since Email::Send is "deprecated" by the maintainer (see: https://metacpan.org/pod/Email::Send#WAIT!-ACHTUNG!), we could do well to replace Email::Send in other code with Email::Sender::Simple. That's a totally different bug, of course.

Whatever modules stay, they should be added to the prerequisites installation.

libdatetime-format-duration-perl is a deb on all of the Debian and Ubuntu releases currently supported by the Evergreen community. The others do not have packages and need to be installed via CPAN.

Why is the documentation under the development category? Shouldn't it be under cataloging or possibly admin?

Change git://git.esilibrary.com/migration-tools.git to https://github.com/EquinoxOpenLibraryInitiative/migration-tools.git. This is the new home of the tools.

The crontab examples should be changed to remove the sourcing of .bashrc and to not cd to /openils/bin. We should encourage people to set up proper crontab files with the environment set appropriately. This means that we should also fix our other example crontab files as well. (That's another bug that I've been meaning to open for some time.)

The instructions should tell the user how to install the program to /openils/bin, or it should be installed as part of make install.--I favor installing it automatically.

I also think that the script and configuration file should be moved out of the Open-ILS/src/support-scripts/bib_magic_importer subdirectory. The script should go in the parent directory (Open-ILS/src/support-scripts) and the configuration example should be moved to Open-ILS/examples. The example configuration should also be installed to SYSCONFDIR as part of the make install process.

To fit better with MARC/LoC nomenclature, change "marc_edit_standard" to "marc_edit_data" as the proper name for these is "Data Field." There is no "Standard Field" in the MARC documentation.

I'll follow up in a few days once I've had a chance to try the code out.