update collections APIs to use package identity #3743
Conversation
tomerd
requested review from
abertelrud,
friedbunny and
neonichu
as code owners
tomerd
force-pushed
the
refactor/package-location-03
branch
from
5e89a8d to
4b88697
Compare
motivation: adoption of SE-0292 package registry chnages: * migrate APIs that take PackageRefernec to take PackageIdentity * adjust and update tests rdar://83073308 rdar://82954076
tomerd
force-pushed
the
refactor/package-location-03
branch
from
4b88697 to
5b7ed39
Compare
|
@swift-ci please smoke test |
| /// Package's repository address | ||
| public let repository: RepositorySpecifier | ||
| /// Package location | ||
| public let location: String | ||
|
|
There was a problem hiding this comment.
We need to keep reference and repository until folks switch over to identity and location.
There was a problem hiding this comment.
@yim-lee on the Package model?
There was a problem hiding this comment.
All of the models. rdar://83113368, rdar://83114538 need to be done before we can remove reference and repository.
| public let identity: PackageIdentity | ||
|
|
||
| /// Package's location | ||
| public let location: String | ||
|
|
There was a problem hiding this comment.
Ditto ^. Cannot remove repository until rdar://83113368 and rdar://83114538 are done.
|
@swift-ci please smoke test |
| switch findPackageResult { | ||
| case .failure(let error): | ||
| callback(.failure(error)) | ||
| case .success(let packagesCollections): | ||
| // A package identity can be associated with multiple repository URLs | ||
| let matches = packagesCollections.packages.filter { $0.reference.canonicalLocation == reference.canonicalLocation } | ||
| let matches = packagesCollections.packages.filter { $0.identity == identity } |
There was a problem hiding this comment.
I think this filtering is no longer needed because $0.identity == identity should be true for all the items in the result.
If we are confident that identity offers "enough" uniqueness, we can potentially revert some of the changes we made in #3543, but at the minimum we can remove this filter.
There was a problem hiding this comment.
oh interesting, I missed that point. we could also add the location as an argument here and continue to do the canonical check. wdyt?
There was a problem hiding this comment.
but in general... the idea of identity is that its unique, so if two collections offer different packages with the same identity, there is def something a bit wrong going on
There was a problem hiding this comment.
the idea of identity is that its unique, so if two collections offer different packages with the same identity, there is def something a bit wrong going on
Right, that was the original assumption.
we could also add the location as an argument here and continue to do the canonical check. wdyt?
Yes, I think I would be more comfortable with this until we completely switch over to identity.
|
@swift-ci please smoke test |
tomerd
added
the
ready
motivation: adoption of SE-0292 package registry
chnages:
rdar://83073308
rdar://82954076