= Review (NS-154): Wayback indexer component =
|| Author || Colin ||
|| Moderator || Colin ||
|| State || Closed ||
== Objectives ==
{{{
This component creates an unsorted cdx file for each file in the archive.
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
time used jolf - 1md
csr 16md
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||
|| Some of the logging levels are too low. The timer thread should log at INFO each time it runs and we should consider adding INFO level logging for each file indexed. || Minor || OK ||


=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/GenericHibernateDAO.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 36-37 || finish javadoc || Cosmetic || OK ||
|| 41 || javadoc || Cosmetic || OK ||
|| 51 || javadoc || Cosmetic || OK ||
|| 60 || javadoc || Cosmetic || OK ||
|| 67 || javadoc || Cosmetic || OK ||
|| 75 || javadoc || Cosmetic || OK ||
|| 91 || javadoc || Cosmetic || OK ||
|| 109-111 || unnecessary empty lines || Cosmetic || OK ||
=== Comments on file 'trunk/build.xml', revision 1391 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 123-136 || Get Søren to accept these new external libraries. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/WaybackIndexer.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 74 || javadoc || Cosmetic || OK ||
|| 85 || javadoc || Cosmetic || OK ||
|| 106 || javadoc || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/ArchiveFile.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 93 || javadoc || Cosmetic || OK ||
|| 98 || javadoc || Cosmetic || OK ||
|| 102 || javadoc || Cosmetic || OK ||
|| 106 || javadoc || Cosmetic || OK ||
|| 110 || javadoc || Cosmetic || OK ||
|| 114 || javadoc || Cosmetic || OK ||
|| 118 || javadoc || Cosmetic || OK ||
|| 127 || What does that mean? || NA || OK ||
|| 132 || javadoc || Cosmetic || OK ||
|| 136 || javadoc || Cosmetic || OK ||
|| 140 || javadoc || Cosmetic || OK ||
|| 144 || javadoc || Cosmetic || OK ||
|| 148 || javadoc || Cosmetic || OK ||
|| 182-188 || comment about what is done here! why only one file? || Cosmetic || OK ||
|| 191 || javadoc || Cosmetic || OK ||
|| 192-209 || Please use some empty lines and comments in this function. It is currently quite difficult to read. || Cosmetic || OK ||
|| 212 || javadoc || Cosmetic || OK ||
|| 230 || javadoc || Cosmetic || OK ||
|| 267 || javadoc || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/HibernateUtil.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 68-104 || All property names should be contants. || Cosmetic || OK ||
|| 98-105 || Can the settings be empty? What would happen? And why is this the only settings, which are tested for whether they are empty?  Write comment about it! || Cosmetic || OK ||
|| 117 || javadoc || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/ArchiveFileDAO.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 37 || javadoc || Cosmetic ||Postponed ||
|| 62 || odd indentation || Cosmetic ||Postponed ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/NetarchiveResourceStore.java', revision 1391 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 216 || javadoc || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/IndexerQueue.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 67-68 || Javadoc || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/WaybackIndexerApplication.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 26 || javadoc class || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/indexer/FileNameHarvester.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 65-66 || no need to create a variable for the instance of the batchjob. Just: client.batch(new FileListJob(), ... || Cosmetic || OK ||
|| 74-81 || What is done if the line already exists? nothing.  Do something, or at least make a comment about it. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/wayback/WaybackSettings.java', revision 1407 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| General || This only contains sporadic javadoc, which deviates from our code-style. || Cosmetic || Postponed ||