Differences between revisions 2 and 3
⇤ ← Revision 2 as of 2010-02-08 11:00:22 →
Size: 8507
Editor: JonasFrellesen
Comment:
← Revision 3 as of 2010-02-08 11:23:27 → ⇥
Size: 8459
Editor: JonasFrellesen
Comment:
Deletions are marked like this. Additions are marked like this.
Line 37: Line 37:
|| 85 || Remove. This will not be implemented. || Cosmetic || NOTOK ||
|| 138-139 || Split up into multiple commands || Cosmetic || NOTOK ||
|| 159-160 || Split up into multiple commands || Cosmetic || NOTOK ||
|| 446 || Describe, when this will be implemented || Cosmetic || NOTOK ||
|| 469 || Describe, when this will be implemented || Cosmetic || NOTOK ||
|| 515, 519 || Does not throw an UnknownID (or IOFailure? - perhaps indirectly) || Cosmetic || NOTOK ||
|| 601 || Add when this will be implemented || Cosmetic || NOTOK ||
|| 641 || typo: teh => the || Cosmetic || NOTOK ||		 || 85 || Remove. This will not be implemented. || Cosmetic || OK ||
|| 138-139 || Split up into multiple commands || Cosmetic || OK ||
|| 159-160 || Split up into multiple commands || Cosmetic || OK ||
|| 446 || Describe, when this will be implemented || Cosmetic || OK ||
|| 469 || Describe, when this will be implemented || Cosmetic || OK ||
|| 515, 519 || Does not throw an UnknownID (or IOFailure? - perhaps indirectly) || Cosmetic || OK ||
|| 601 || Add when this will be implemented || Cosmetic || OK ||
|| 641 || typo: teh => the || Cosmetic || OK ||
Line 47: Line 47:
|| 358-359 || Change to "Checksum job message submitted for file '" + filename + "' with message id '" + msg.getID + "' || Cosmetic || NOTOK || || 358-359 || Change to "Checksum job message submitted for file '" + filename + "' with message id '" + msg.getID + "' || Cosmetic || OK ||
Line 50: Line 50:
|| 72 || Does "incorrectChecksum" refer to an undefined variable? || Cosmetic || NOTOK || || 72 || Does "incorrectChecksum" refer to an undefined variable? || Cosmetic || OK ||
Line 53: Line 53:
|| 60-61 || What prevents you from removing this constructor, now? || Cosmetic || NOTOK ||
|| 98 || Typo: "fopr .." => "for the file" || Cosmetic || NOTOK ||
|| 101-102 || Add missing argument-check || Cosmetic || NOTOK ||		 || 60-61 || What prevents you from removing this constructor, now? || Cosmetic || OK ||
|| 98 || Typo: "fopr .." => "for the file" || Cosmetic || OK ||
|| 101-102 || Add missing argument-check || Cosmetic || OK ||
Line 58: Line 58:
|| 66 || change "theCR" to "theChecksumChannel" or "theChecksumQueue" || Cosmetic || NOTOK ||
|| 192 || What is this TODO about? I don't understand it || Cosmetic || NOTOK ||
|| 192 || Remove! The todo is fixed || Cosmetic || NOTOK ||		 || 66 || change "theCR" to "theChecksumChannel" or "theChecksumQueue" || Cosmetic || OK ||
|| 192 || What is this TODO about? I don't understand it || Cosmetic || OK ||
|| 192 || Remove! The todo is fixed || Cosmetic || OK ||

Review (NS-145): Assignment B.2.2b: Make bitarchive replica use new messages.

Author

Jonas

Moderator

Jonas

State

Review

Objectives

See FR 1823.

Total Time Used (Coding,Documentation,Review): 

Time use (Coding,Documentation,Review)
SVC: 1.0
JOLF: 5.0

General comments:

Description

Classification

Status

Use different termilogy: wrong/bad => corrupt

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumFileServer.java', revision 1246

Lines

Description

Classification

Status

263

Should we validate the badFile before sending it back?

Cosmetic

REJECT

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/distribute/JMSArcRepositoryClient.java', revision 1246

Lines

Description

Classification

Status

684

Replace 'wrong' with 'corrupt'

Cosmetic

OK

717, 721

This should be 'removedFile' instead of retrieving the variable again.

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/CorrectMessage.java', revision 1246

Lines

Description

Classification

Status

79

"String checksum" => "String badChecksum"

Cosmetic

OK

173-176

Issue warning, and return null.

Minor

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/FileChecksumArchive.java', revision 1246

Lines

Description

Classification

Status

723-724

Language: Change to "Make the file containing the corrupt entry be returned in the CorrectMessage

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/DatabaseBasedActiveBitPreservation.java', revision 1254

Lines

Description

Classification

Status

85

Remove. This will not be implemented.

Cosmetic

OK

138-139

Split up into multiple commands

Cosmetic

OK

159-160

Split up into multiple commands

Cosmetic

OK

446

Describe, when this will be implemented

Cosmetic

OK

469

Describe, when this will be implemented

Cosmetic

OK

515, 519

Does not throw an UnknownID (or IOFailure? - perhaps indirectly)

Cosmetic

OK

601

Add when this will be implemented

Cosmetic

OK

641

typo: teh => the

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/ArcRepository.java', revision 1242

Lines

Description

Classification

Status

358-359

Change to "Checksum job message submitted for file '" + filename + "' with message id '" + msg.getID + "'

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/ChecksumArchive.java', revision 1246

Lines

Description

Classification

Status

72

Does "incorrectChecksum" refer to an undefined variable?

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/RemoveAndGetFileMessage.java', revision 1246

Lines

Description

Classification

Status

60-61

What prevents you from removing this constructor, now?

Cosmetic

OK

98

Typo: "fopr .." => "for the file"

Cosmetic

OK

101-102

Add missing argument-check

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumClient.java', revision 1247

Lines

Description

Classification

Status

66

change "theCR" to "theChecksumChannel" or "theChecksumQueue"

Cosmetic

OK

192

What is this TODO about? I don't understand it

Cosmetic

OK

192

Remove! The todo is fixed

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveMonitorServer.java', revision 1251

Lines

Description

Classification

Status

107, 110, 116

"First" => "In the first stage" "Second" => In the second stage" "Third" => "In the third stage"

Cosmetic

NOTOK

112

an UploadMessage

Cosmetic

NOTOK

240-251

This description here is redundant

Cosmetic

NOTOK

258

Typo: Recieving => Receiving

Cosmetic

NOTOK

258, 298-299

Receiving

Cosmetic

NOTOK

280-281

Redundant description

Cosmetic

NOTOK

289-291

Redundant description

Cosmetic

NOTOK

308

Typo: filed => failed

Cosmetic

NOTOK

333-340

redundant description

Cosmetic

NOTOK

368, 371

Finish the two sentences.

NA

NOTOK

373

argument validation

Cosmetic

NOTOK

389

argument validation + javadoc

Cosmetic

NOTOK

405

argument validation + javadoc.

Cosmetic

NOTOK

444

Trouble while handling batch request => Unable to handle batch request due to unexpected exception

Cosmetic

NOTOK

561

Which other exceptions are foreseen here besides the IOFailure thrown in case of unhandled message type? The called method handle their own exceptions, so none. The 'try - catch' is redundant if the reply is put into the last 'else' clause.

Cosmetic

NOTOK

640

Change to: Fetch the contents of the batchresultfile

Cosmetic

NOTOK

657

Include the others in the log entry

Cosmetic

NOTOK

664

explain what valid means here

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/FileBasedActiveBitPreservation.java', revision 1254

Lines

Description

Classification

Status

296

This method is redundant: the method only forwards to the getChecksum method

Cosmetic

NOTOK

347

Wrong spelling of 'successful'

Cosmetic

NOTOK

363

Wrong checkNotNull message

Cosmetic

NOTOK

402

Replace log message with "Finding missing files in directory '"

Cosmetic

NOTOK

413

Add to comment what difference set 1 is

Cosmetic

NOTOK

431

Add to comment what difference set 2 is

Cosmetic

NOTOK

449

Replace log message with "Finished finding missing files"

Cosmetic

NOTOK

472-474

Make an external variable for value of "ArcRepositoryClientFactory.getPreservationInstance().getAllFilenames(replica.getId))" so this value can be checked before calling the copyFile method

Cosmetic

NOTOK

478

Don't use the word 'wrong'. Use the word 'corrupt' instead.

Cosmetic

NOTOK

619-620

Make an external variable for value of "ArcRepositoryClientFactory.getPreservationInstance().getAllChecksums(replica.getId))" so this value can be checked before calling the copyFile method

Cosmetic

NOTOK

718-720

Please change to: Check that the files we want to restore are indeed missing on the replica on which we are restoring them, and presnt in admin data and the reference replica. If so, upload the missing files from the reference replica to this replica.

Cosmetic

NOTOK

873

Add that: We don't wait for an acknowledgement that admin data has indeed been updated.

Cosmetic

NOTOK

916

change "a bad entry" to "an entry"

Cosmetic

NOTOK

962

When will this be implemented?

Cosmetic

NOTOK

975

When will this be implemented?

Cosmetic

NOTOK

988

Change to "String... filenames"

Cosmetic

NOTOK

990

When will this be implemented?

Cosmetic

NOTOK

1023

When will this be implemented?

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveClient.java', revision 1247

Lines

Description

Classification

Status

229-230

Remove this @throws clause

Cosmetic

NOTOK

243

why was this line added?

Cosmetic

NOTOK

247

Typo: be send => be sent

Cosmetic

NOTOK

286-288

Document that a Argument not valid is thrown

Cosmetic

NOTOK

313

remove

Cosmetic

NOTOK

IssuesFromNs145 (last edited 2010-08-16 10:24:44 by localhost)