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 |