= Review (MG-3): Changed java package names, what a mess in Intellij. =
|| Author || Henrik Kirk ||
|| Moderator || Henrik Kirk ||
|| State || Review ||
== Objectives ==
{{{
MAIN:hbk:20090929112545: Changed java package names, what a mess in Intellij.
}}}
'''Total Time Used (Coding,Documentation,Review)''':
{{{ 
Time use (Coding,Documentation,Review) 
HBK: 0.5 MD
SVC: 1MD
}}}

'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||
|| Use the following fileheader when adding fileheaders to your CVS files
/* $RCSfile$
 * $Revision$
 * $Date$
 * $Author$
 *
 * The Netarchive Suite - Software to harvest and preserve websites
 * Copyright 2004-2007 Det Kongelige Bibliotek and Statsbiblioteket, Denmark
 *
 * This library is free software; you can redistribute it and/or
 * modify it under the terms of the GNU Lesser General Public
 * License as published by the Free Software Foundation; either
 * version 2.1 of the License, or (at your option) any later version.
 *
 * This library is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 * Lesser General Public License for more details.
 *
 * You should have received a copy of the GNU Lesser General Public
 * License along with this library; if not, write to the Free Software
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 */ || Cosmetic || OK ||
|| You need to write @param software Software used for this harvest/crawl
instead of just @param software used for this harvest/crawl.

Try generating the javadoc, and see my point. || Cosmetic || OK ||

=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/archiveConverters/Archive2WARC.java', revision 1.3 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 57 || Indicate what the suffix of the files generated from the Warc-writer is? Should this suffix be a parameter to this method. || Cosmetic || OK ||
|| 69 || What constructure are we talking about. This abstract class does not have any constructures. || Cosmetic || OK ||
|| 70-72 || The IllegalArgumentException is currently thrown if the parameter "af" is null. Does warcW.writeResponseRecord also throw IllegalArgumentException? || Cosmetic || OK ||
|| 78 || Change to "no instance of ConverterWARCWriter created. Call OpenWARCStream first." || Cosmetic || OK ||
|| 83 || spelling: diffenrent => different || Cosmetic || OK ||
|| 96-105 || You need to write @param software Software used for this harvest/crawl and similarly for the other @param lines. || Cosmetic || OK ||
|| 117 || See previous comment || Cosmetic || OK ||
|| 124 || Spelling: haeders => headers || Cosmetic || OK ||
|| 149 || Change to @throws IOException If exception is caught during the writing of the WarcInfoRecord, or if the WARC Info record have already been written and forceInfoWrite is set to false. || Cosmetic || OK ||
|| 162 || Change to: Closes the WarcConverterWriter || Cosmetic || OK ||
|| 188 || spelling: wehter = > whether || Cosmetic || OK ||
|| 206 || Should we have checks here for validity of the metadata? add as TODO. refer to the specification || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/common/WARCDate.java', revision 1.6 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 27 || Don't you mean GMT (Greanwich Mean Time)? || Cosmetic || OK ||
|| 47 || Add a period at end of line || Cosmetic || OK ||
|| 48 || spelling: contaning => containing || Cosmetic || OK ||
|| 50 || spelling: formate => format || Cosmetic || OK ||
|| 68 || Add that the the string looks like this: 2008-12-19T23:22:43Z  format = YYYY-MM-DDTHH:mm:ssZ || Cosmetic || OK ||
|| 85 || add missing period at end of line || Cosmetic || OK ||
|| 87 || Add or null if lastmodified <= 0 (indicating that the file does not exist?) || Cosmetic || OK ||
|| 101, 103 || Add missing period at end of line || Cosmetic || OK ||
|| 105, 109 || Write in javadoc and in the exception that the number (X) is must be in the interval [0,60] || Cosmetic || OK ||
|| 126 || Don't you want to check that the input has the expected format? add TODO to make a regexp for this format and check the string up against the regexp || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/archiveConverters/io/ArchiveRecord.java', revision 1.2 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 37, 44, 51, 57, 66, 72, 80, 86, 92, 98, 104, 110, 116, 123 || The public modifier is redundant. In a public interface, all methods are public by default || Cosmetic || OK ||
|| 95 || spelling: Retrurn => return || Cosmetic || OK ||
|| 119-120 || What does this javadoc sentence mean? || Cosmetic || OK ||
|| 121 || Change to @return the WARCrecordType for this ArchiveRecord || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/archiveConverters/io/AbstractArchiveRecord.java', revision 1.2 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 30, 32 || Should these fields be optional? Or delete these lines || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/archiveConverters/io/Constants.java', revision 1.1 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 30-39 || Add javadoc for these constants || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/archiveConverters/ConverterWARCWriter.java', revision 1.2 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 47 || What does the string mean? || Cosmetic || OK ||
|| 69 || language: if the underlaying WARC record fails => If the writing of the WARC response record fails. || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/common/CoolStringMethods.java', revision 1.3 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 37 || Missing period || NA || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/common/io/CopyFile.java', revision 1.4 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 35 || Copy file. || Cosmetic || OK ||
|| 43 || Add a finally clause that close the streams || Cosmetic || OK ||
=== Comments on file 'hbk/ArchiveToWARC/src/dk/sb/migration/common/io/FileUtils.java', revision 1.3 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 38 || Add: If the given directory is a file, a list with this file included will be returned || Cosmetic || OK ||