10335
Comment:
|
← Revision 3 as of 2010-08-16 10:24:44 ⇥
10121
converted to 1.6 markup
|
Deletions are marked like this. | Additions are marked like this. |
Line 7: | Line 7: |
A component to allow lists of crawler traps to be appended to all scheduled jobs. There are | A component to allow lists of crawler traps to be appended to all scheduled jobs. There are |
Line 15: | Line 15: |
|| Append the english versions of the new properties to the other language files (Translations_de.properties, Translations_fr.properties, Translations_it.properties) || Cosmetic || NOTOK || || Remove @author and @since tags in all the code-files. We don't use these tags in NetarchiveSuite || Cosmetic || NOTOK || || Time Used svc: 1md csr: 12md || Cosmetic || NOTOK || |
|| Append the english versions of the new properties to the other language files (Translations_de.properties, Translations_fr.properties, Translations_it.properties) || Cosmetic || OK || || Remove @author and @since tags in all the code-files. We don't use these tags in NetarchiveSuite || Cosmetic || OK || |
Line 21: | Line 18: |
|| Time Used svc: 1md csr: 12md || Cosmetic || NOTOK || |
|
Line 24: | Line 23: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 39-64 || Add javadoc before each enum value || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 39-64 || Add javadoc before each enum value || Cosmetic || OK || |
Line 28: | Line 27: |
|| General || Add NetarchiveSuite header-file || Cosmetic || NOTOK || || 24 || long line; make newline before 'value=..' || Cosmetic || NOTOK || |
|| General || Add NetarchiveSuite header-file || Cosmetic || OK || || 24 || long line; make newline before 'value=..' || Cosmetic || OK || |
Line 32: | Line 31: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 45 || Complete the @return statement || Cosmetic || NOTOK || || 51 || Complete the @return statement || Cosmetic || NOTOK || || 58 || Complete the @return statement || Cosmetic || NOTOK || || 75-80 || Missing javadoc for these methods || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 45 || Complete the @return statement || Cosmetic || OK || || 51 || Complete the @return statement || Cosmetic || OK || || 58 || Complete the @return statement || Cosmetic || OK || || 75-80 || Missing javadoc for these methods || Cosmetic || OK || |
Line 39: | Line 38: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 108-111, 113-114, 117-125 || Shouldn't line 108-125 be put into a try .. catch so that any database errors are caught? || Cosmetic || NOTOK || || 115 || This could be written more concisely like: if (fileName != null && !fileName.isEmpty()) || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 108-111, 113-114, 117-125 || Shouldn't line 108-125 be put into a try .. catch so that any database errors are caught? || Cosmetic || OK || || 115 || This could be written more concisely like: if (fileName != null && !fileName.isEmpty()) || Cosmetic || OK || |
Line 44: | Line 43: |
|| 394 || Missing javadoc for this method || Cosmetic || NOTOK || || 401 || Remove redundant line || Cosmetic || NOTOK || |
|| 394 || Missing javadoc for this method || Cosmetic || OK || || 401 || Remove redundant line || Cosmetic || OK || |
Line 48: | Line 47: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 56 || say which table it is unique in. || Cosmetic || NOTOK || || 58-60 || Missing javadoc || Cosmetic || NOTOK || || 62-64 || Missing javadoc Missing argument validation (not null and not empty string) || Cosmetic || NOTOK || || 67 || Missing '.' at end of line. || NA || NOTOK || || 69 || say which table it is unique in. || Cosmetic || NOTOK || || 132-133 || Missing '.' at end of line. || Cosmetic || NOTOK || || 136 || Missing argument validation of argument 'is' || Cosmetic || NOTOK || || 149-151 || Missing javadoc || Cosmetic || NOTOK || || 153 || Missing javadoc || NA || NOTOK || || 157 || Missing javadoc || NA || NOTOK || || 161 || Missing javadoc Missing argument validation || Cosmetic || NOTOK || || 165 || Missing javadoc || Cosmetic || NOTOK || || 169 || Missing javadoc Missing argument validation || Cosmetic || NOTOK || || 173 || Missing javadoc || Cosmetic || NOTOK || || 177-179 || Missing javadoc || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 56 || say which table it is unique in. || Cosmetic || OK || || 58-60 || Missing javadoc || Cosmetic || OK || || 62-64 || Missing javadoc Missing argument validation (not null and not empty string) || Cosmetic || OK || || 67 || Missing '.' at end of line. || NA || OK || || 69 || say which table it is unique in. || Cosmetic || OK || || 132-133 || Missing '.' at end of line. || Cosmetic || OK || || 136 || Missing argument validation of argument 'is' || Cosmetic || OK || || 149-151 || Missing javadoc || Cosmetic || OK || || 153 || Missing javadoc || NA || OK || || 157 || Missing javadoc || NA || OK || || 161 || Missing javadoc Missing argument validation || Cosmetic || OK || || 165 || Missing javadoc || Cosmetic || OK || || 169 || Missing javadoc Missing argument validation || Cosmetic || OK || || 173 || Missing javadoc || Cosmetic || OK || || 177-179 || Missing javadoc || Cosmetic || OK || |
Line 66: | Line 65: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 42-43 || Remove @author and @since tags. We don't use these tags in NetarchiveSuite || Cosmetic || NOTOK || || 58 || Breaks checkstyle rule: move "+" at end of line to next line. || Cosmetic || NOTOK || || 64 || Insert whitespace around ":" || Cosmetic || NOTOK || || 65 || replace (trap+"\n") with (trap + "\n") || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 42-43 || Remove @author and @since tags. We don't use these tags in NetarchiveSuite || Cosmetic || OK || || 58 || Breaks checkstyle rule: move "+" at end of line to next line. || Cosmetic || OK || || 64 || Insert whitespace around ":" || Cosmetic || OK || || 65 || replace (trap+"\n") with (trap + "\n") || Cosmetic || OK || |
Line 73: | Line 72: |
|| General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || NOTOK || | || General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || OK || |
Line 76: | Line 75: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 110 || Missing javadoc || Cosmetic || NOTOK || || 113 || Missing javadoc; add new line before method || Cosmetic || NOTOK || || 127 || Move "+" to next line || Cosmetic || NOTOK || || 129 || Is it really necessary to log here? The exception is rethrown just after the logentry || Cosmetic || NOTOK || || 144 || Add missing javadoc || Cosmetic || NOTOK || || 149 || add newline || Cosmetic || NOTOK || || 168 || Add missing javadoc Remove long line || Cosmetic || NOTOK || || 214-219 || Add javadoc || Cosmetic || NOTOK || || 226 || Add comment that here we delete all entries from global_crawler_trap_lists where .... || Cosmetic || NOTOK || || 229 || Add comment that here we delete all entries from global_crawler_trap_expressions where .... || Cosmetic || NOTOK || || 244 || Add javadoc || Cosmetic || NOTOK || || 249 || Add missing argument validation || Cosmetic || NOTOK || || 254 || Add comment to what is done here || Cosmetic || NOTOK || || 260 || Add comment to what is done here || Cosmetic || NOTOK || || 263 || Add comment to what is done here || Cosmetic || NOTOK || || 271 || Move "+" to next line || Cosmetic || NOTOK || || 281-286 || Add javadoc || Cosmetic || NOTOK || || 296 || Move "+" to next line || Cosmetic || NOTOK || || 312 || Move "+" to next line || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic ||OK || || 110 || Missing javadoc || Cosmetic || OK || || 113 || Missing javadoc; add new line before method || Cosmetic || OK || || 127 || Move "+" to next line || Cosmetic || OK || || 129 || Is it really necessary to log here? The exception is rethrown just after the logentry || Cosmetic || OK || || 144 || Add missing javadoc || Cosmetic || OK || || 149 || add newline || Cosmetic || OK || || 168 || Add missing javadoc Remove long line || Cosmetic || OK || || 214-219 || Add javadoc || Cosmetic || OK || || 226 || Add comment that here we delete all entries from global_crawler_trap_lists where .... || Cosmetic || OK || || 229 || Add comment that here we delete all entries from global_crawler_trap_expressions where .... || Cosmetic || OK || || 244 || Add javadoc || Cosmetic || OK || || 249 || Add missing argument validation || Cosmetic || OK || || 254 || Add comment to what is done here || Cosmetic || OK || || 260 || Add comment to what is done here || Cosmetic || OK || || 263 || Add comment to what is done here || Cosmetic || OK || || 271 || Move "+" to next line || Cosmetic || OK || || 281-286 || Add javadoc || Cosmetic || OK || || 296 || Move "+" to next line || Cosmetic || OK || || 312 || Move "+" to next line || Cosmetic || OK || |
Line 98: | Line 97: |
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || | || General || Add NetarchiveSuite fileheader || Cosmetic || OK || |
Line 101: | Line 100: |
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || | || General || Add NetarchiveSuite fileheader || Cosmetic || OK || |
Line 104: | Line 103: |
|| General || Missing standard NetarchiveSuite header-file || Cosmetic || NOTOK || | || General || Missing standard NetarchiveSuite header-file || Cosmetic || OK || |
Line 107: | Line 106: |
|| General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || NOTOK || || 320 || Is the case not wrong here? || Cosmetic || NOTOK || |
|| General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || OK || || 320 || Is the case not wrong here? || Cosmetic || OK || |
Line 111: | Line 110: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 70, 77 || Replace "".equals(requestType) with requestType.isEmpty() || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 70, 77 || Replace "".equals(requestType) with requestType.isEmpty() || Cosmetic || OK || |
Line 115: | Line 114: |
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || | || General || Add NetarchiveSuite fileheader || Cosmetic || OK || |
Line 118: | Line 117: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 57 || Shouldn't we put some of this code inside a try..catch, so that we can intercept any database errors? || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 57 || Shouldn't we put some of this code inside a try..catch, so that we can intercept any database errors? || Cosmetic || OK || |
Line 122: | Line 121: |
|| 86-87 || add missing "." || Cosmetic || NOTOK || || 89-99 || Consider adding separate javadoc for each constant || Cosmetic || NOTOK || |
|| 86-87 || add missing "." || Cosmetic || OK || || 89-99 || Consider adding separate javadoc for each constant || Cosmetic ||Rejected|| |
Line 126: | Line 125: |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || NOTOK || || 44-45 || Put into a try-catch, intercept any SQL-exceptions, and forward an error the user || Cosmetic || NOTOK || |
|| General || Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) || Cosmetic || OK || || 44-45 || Put into a try-catch, intercept any SQL-exceptions, and forward an error the user || Cosmetic || OK || |
Review (NS-143): FR1116: Global Crawler Traps
Author |
Colin |
Moderator |
Colin |
State |
Closed |
Objectives
A component to allow lists of crawler traps to be appended to all scheduled jobs. There are three sub-components i. Backend: datamodel + DAO ii. Business logic: (small) modifications to class Job which actually add the global crawler traps to all new Jobs iii. GUI: a single page with corresponding backend for carrying out all the necessary operations for managing lists of global crawler traps - create, read, update, delete and activate/deactivate.
General comments:
Description |
Classification |
Status |
Append the english versions of the new properties to the other language files (Translations_de.properties, Translations_fr.properties, Translations_it.properties) |
Cosmetic |
OK |
Remove @author and @since tags in all the code-files. We don't use these tags in NetarchiveSuite |
Cosmetic |
OK |
Time Used svc: 1md csr: 12md |
Cosmetic |
NOTOK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapActionEnum.java', revision 1226
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
39-64 |
Add javadoc before each enum value |
Cosmetic |
OK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist-download-element.jspf', revision 1239
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite header-file |
Cosmetic |
OK |
24 |
long line; make newline before 'value=..' |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapListDAO.java', revision 1226
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
45 |
Complete the @return statement |
Cosmetic |
OK |
51 |
Complete the @return statement |
Cosmetic |
OK |
58 |
Complete the @return statement |
Cosmetic |
OK |
75-80 |
Missing javadoc for these methods |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapCreateOrUpdateAction.java', revision 1227
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
108-111, 113-114, 117-125 |
Shouldn't line 108-125 be put into a try .. catch so that any database errors are caught? |
Cosmetic |
OK |
115 |
This could be written more concisely like: if (fileName != null && !fileName.isEmpty()) |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/Job.java', revision 1226
Lines |
Description |
Classification |
Status |
394 |
Missing javadoc for this method |
Cosmetic |
OK |
401 |
Remove redundant line |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapList.java', revision 1226
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
56 |
say which table it is unique in. |
Cosmetic |
OK |
58-60 |
Missing javadoc |
Cosmetic |
OK |
62-64 |
Missing javadoc Missing argument validation (not null and not empty string) |
Cosmetic |
OK |
67 |
Missing '.' at end of line. |
NA |
OK |
69 |
say which table it is unique in. |
Cosmetic |
OK |
132-133 |
Missing '.' at end of line. |
Cosmetic |
OK |
136 |
Missing argument validation of argument 'is' |
Cosmetic |
OK |
149-151 |
Missing javadoc |
Cosmetic |
OK |
153 |
Missing javadoc |
NA |
OK |
157 |
Missing javadoc |
NA |
OK |
161 |
Missing javadoc Missing argument validation |
Cosmetic |
OK |
165 |
Missing javadoc |
Cosmetic |
OK |
169 |
Missing javadoc Missing argument validation |
Cosmetic |
OK |
173 |
Missing javadoc |
Cosmetic |
OK |
177-179 |
Missing javadoc |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapReadAction.java', revision 1227
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
42-43 |
Remove @author and @since tags. We don't use these tags in NetarchiveSuite |
Cosmetic |
OK |
58 |
Breaks checkstyle rule: move "+" at end of line to next line. |
Cosmetic |
OK |
64 |
Insert whitespace around ":" |
Cosmetic |
OK |
65 |
replace (trap+"\n") with (trap + "\n") |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/MySQLSpecifics.java', revision 1238
Lines |
Description |
Classification |
Status |
General |
Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapListDBDAO.java', revision 1226
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
110 |
Missing javadoc |
Cosmetic |
OK |
113 |
Missing javadoc; add new line before method |
Cosmetic |
OK |
127 |
Move "+" to next line |
Cosmetic |
OK |
129 |
Is it really necessary to log here? The exception is rethrown just after the logentry |
Cosmetic |
OK |
144 |
Add missing javadoc |
Cosmetic |
OK |
149 |
add newline |
Cosmetic |
OK |
168 |
Add missing javadoc Remove long line |
Cosmetic |
OK |
214-219 |
Add javadoc |
Cosmetic |
OK |
226 |
Add comment that here we delete all entries from global_crawler_trap_lists where .... |
Cosmetic |
OK |
229 |
Add comment that here we delete all entries from global_crawler_trap_expressions where .... |
Cosmetic |
OK |
244 |
Add javadoc |
Cosmetic |
OK |
249 |
Add missing argument validation |
Cosmetic |
OK |
254 |
Add comment to what is done here |
Cosmetic |
OK |
260 |
Add comment to what is done here |
Cosmetic |
OK |
263 |
Add comment to what is done here |
Cosmetic |
OK |
271 |
Move "+" to next line |
Cosmetic |
OK |
281-286 |
Add javadoc |
Cosmetic |
OK |
296 |
Move "+" to next line |
Cosmetic |
OK |
312 |
Move "+" to next line |
Cosmetic |
OK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_createorupdate_element.jspf', revision 1239
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
OK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_activation_element.jspf', revision 1238
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
OK |
Comments on file 'trunk/webpages/HarvestDefinition/Definitions-edit-global-crawler-traps.jsp', revision 1239
Lines |
Description |
Classification |
Status |
General |
Missing standard NetarchiveSuite header-file |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/DerbySpecifics.java', revision 1233
Lines |
Description |
Classification |
Status |
General |
Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() |
Cosmetic |
OK |
320 |
Is the case not wrong here? |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapAction.java', revision 1226
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
70, 77 |
Replace "".equals(requestType) with requestType.isEmpty() |
Cosmetic |
OK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_delete_element.jspf', revision 1238
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapActivationAction.java', revision 1227
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
57 |
Shouldn't we put some of this code inside a try..catch, so that we can intercept any database errors? |
Cosmetic |
OK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/Constants.java', revision 1227
Lines |
Description |
Classification |
Status |
86-87 |
add missing "." |
Cosmetic |
OK |
89-99 |
Consider adding separate javadoc for each constant |
Cosmetic |
Rejected |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapDeleteAction.java', revision 1209
Lines |
Description |
Classification |
Status |
General |
Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id) |
Cosmetic |
OK |
44-45 |
Put into a try-catch, intercept any SQL-exceptions, and forward an error the user |
Cosmetic |
OK |