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]
