On 22/06/07, Kenney Westerhof <[EMAIL PROTECTED]> wrote:
Well, just looked at the code and it's a mess. Here are the problems:
* The isSnapshot method does a regexp on the version for the timestamp, and if
it matches,
replaces the baseversion with -SNAPSHOT. If it doesn't match, checks for
'endswith(SNAPSHOT)'.
So this method changes the baseversion on some occasions.
* The above means that calling isSnapshot on an artifact with baseversion
'1SNAPSHOT' or 'SNAPSHOT' is
considered a snapshot IFF the baseversion is not timestamp-resolved yet.
So, it'll return true for non-timestamp resolved artifacts, but
it'll return false if they have. This is because the regexp checks for
^(.*)-(\d\d\detc....)',
so the timestamp has to be preceded by '-'.
* LATEST is also considered a snapshot, if the version has not been resolved.
If it has been resolved
to a real version (1.0 for instance), it won't be a snapshot. I'm not sure if
this is a problem,
if only snapshots are considered in resolution. Shouldn't LATEST resolve to
non-snapshots?
Ah, they do: AbstractVersionTransformation:
// Don't use snapshot metadata for LATEST (which isSnapshot returns
true for)
if ( !artifact.isSnapshot() || Artifact.LATEST_VERSION.equals(
artifact.getBaseVersion() ) )
{
metadata = new ArtifactRepositoryMetadata( artifact );
}
else
{
metadata = new SnapshotArtifactRepositoryMetadata( artifact );
}
This needs to be fixed - i'm sure there's a reason for LATEST being treated
like snapshots at some
places, and not at later places. As soon as LATEST is resolved it's no longer
a snapshot, surely this
causes problems?
* There are multiple methods for version modification/retrieval:
* resolved version (setter only - sets version)
* version (sets version, baseversion, and versionrange; gets version)
* base version (sets base version, gets baseversion or version if baseversion
null)
* version range (sets versionrange, and sets version and baseversion either
to null or a recommeded version; gets versionrange)
* available versions
* selectVersion (sets version and baseversion)
* updateVersion (sets version and file)
These can be set independently and can result in an inconsistent state.
Documentation lacks to describe
which you can and which you cannot call in what sequences.
* isRelease is a boolean that has nothing to do with versions. I'd figure that
if something wasn't a snapshot,
it'd be a release. Guess not. Or maybe it is, but then the logic is someplace
else.
Simplifying this should make a whole lot of difference.
For now, we should update the setters to immediately update baseversion to
-SNAPSHOT,
so that even when you don't call isSNapshot, getBaseVersion is ok. Just
updating setBaseVersion isn't enough,
but it's a start. If you do setBaseVersion(timestamp) it'll discard the real
value and replace it with -SNAPSHOT.
Ideally I'd like to NOT use the regexps to find out if something is a snapshot,
but use the repository
metadata for that. Maybe some company uses timestamps for releases, who knows..
I noticed some of this madness when I was reading the code recently
and am certainly +1 on cleaning up the API and writing some Javadoc.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]