api cleanup in preperation for identity improvements #3214
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tomerd
requested review from
abertelrud,
friedbunny and
neonichu
as code owners
|
replaces some of the work done in #3152. this is the less risky subset of the changes, so we can create a separate PR for the more risky ones |
|
@swift-ci please smoke test |
tomerd
changed the title
neonichu
approved these changes
Jan 21, 2021
| @@ -95,7 +95,7 @@ public enum DependencyResolutionNode { | |||
| // Don’t create a version lock for anything but a product. | |||
| guard specificProduct != nil else { return nil } | |||
| return PackageContainerConstraint( | |||
| container: package, | |||
| package: package, | |||
There was a problem hiding this comment.
Can we call this packageReference?
There was a problem hiding this comment.
@neonichu we could, but in many other places in the code vase it is called "package". I actually have a commit with this change but its large if we want to be consistent. wdyt?
| @@ -24,35 +24,35 @@ import TSCUtility | |||
| /// should be used as-is. Infact, they might not even have a git repository. | |||
| /// Examples: Root packages, local dependencies, edited packages. | |||
| public final class LocalPackageContainer: PackageContainer { | |||
| public let identifier: PackageReference | |||
| public let package: PackageReference | |||
There was a problem hiding this comment.
Can we call this packageReference?
motivation: infrastructure towards improve correctness by using identity more broadly throughout the code changes: * rename PackageReference::path to PackageReference::location * make PackageReference kinf explicitly set by the callsite with no default * rename PackageContainerConstraint::identifier to PackageContainerConstraint::package * rename PackageContainer::identifier to PackageContainer::package * adjust tests
tomerd
force-pushed
the
refactor/identity-01
branch
from
ca6fab3 to
606a9e1
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test linux |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
motivation: infrastructure towards improving package identity correctness
changes: