archiva-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joakim Erdfelt <joa...@erdfelt.com>
Subject Re: [plexus work] archiva-checksums
Date Sat, 12 Apr 2008 02:36:14 GMT
Brett Porter wrote:
> Looks good to me. I say roll it into trunk.
>
> A couple of uncertainties:
> - isn't doFinal required?

doFinal ?

> - I think it can use more tests

Agreed.  Have any special use cases you want fleshed out?

>
>
> Here's some nitpicking that might reduce the amount of code and make 
> it clearer:
> - rename Hash to ChecksumAlgorithm (might as well be consistent and 
> use checksum everywhere instead of alternating through Hash, Digest, 
> Checksum?)
> - likewise Hasher -> Checksum

Name changes make sense, I'll make these changes this weekend.

> - there some duplicated code in isValidChecksums and fixChecksums that 
> can be factored out

Good catch, I'll put that on my todo's too.

> - what about passing referenceFile as a final field, since every 
> method uses it?

I started ChecksumFile as a utility class, but that might not be the 
best decision.
Any opinions if I make ChecksumFile an instantiated class?

ChecksumFile cf = new ChecksumFile(new File("random.jar"));
if( cf.valid() == false ) {
  cf.createChecksum( ChecksumAlgorithm.SHA1 );
}

wdyt?

>   - but if not, it'd be better if the arg order of getChecksumFile and 
> createChecksum and calculateChecksum were consistent (I would go with 
> File first)
> - I'd rename ChecksumFile as ChecksummedFile
> - parseChecksum can be private?

I left that as public just to have it be easily unit tested. ;-)

> - the two isValidChecksum(s) methods do different things. 
> isValidChecksum should be isValidChecksum(Hash hash) { return 
> isValidChecksums( new Hash[] { hash } ); }. Maybe another static 
> convenience method is needed for the checksum file checkChecksumFile( 
> File checksumFile ).

Legitimate fix.  Adding to my todo.

>
> - Brett
>
> On 09/04/2008, at 2:39 PM, Joakim Erdfelt wrote:
>> I've been taking a stab and removing some of our dependencies on 
>> various plexus components.
>>
>> First up, plexus-digest.
>>
>> I've taken the varied code from many locations and come up with a 
>> stand alone archiva-checksum lib/component that I hope to be able to 
>> integrate into archiva/trunk.
>>
>> So ... uhm ... consider this a call for discussion. ;-)
>>
>> - Joakim
>
> -- 
> Brett Porter
> brett@apache.org
> http://blogs.exist.com/bporter/
>


Mime
View raw message