Skip to content

Conversation

@tomerd
Copy link
Contributor

@tomerd tomerd commented Feb 5, 2021

motivation: the logic in Versioning does not belong in TSC, its a SwiftPM concern

changes:

  • move Versioning/SwiftVersion to Basics module
  • move Git::convertTagsToVersionMap to its only call site
  • adjust call sites

motivation: the logic in Versioning does not belong in TSC, its a SwiftPM concern

changes:
* move Versioning/SwiftVersion to Basics module
* move Git::convertTagsToVersionMap to its only callsite
* adjust callsites
@tomerd
Copy link
Contributor Author

tomerd commented Feb 5, 2021

@swift-ci please smoke test

@tomerd tomerd self-assigned this Feb 5, 2021
@tomerd
Copy link
Contributor Author

tomerd commented Feb 5, 2021

@swift-ci please smoke test

@tomerd
Copy link
Contributor Author

tomerd commented Feb 5, 2021

related TSC PR: swiftlang/swift-tools-support-core#188

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great; thank you for dealing with this! Just a question inline but since it builds with self-hosted I'm pretty sure I know the answer; just asked to confirm understanding.

TSCBasic
TSCUtility)
target_link_libraries(Basics PRIVATE
TSCclibc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this is private (and imported using @_implementationOnly), it doesn't introduce a requirement on having the headers for TSCclibc around for any clients of libSwiftPM, right? Getting rid of that was a good cleanup and we'd hate to give it up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that was my plan - its the same setup as TSC has

@tomerd tomerd merged commit 91fac80 into swiftlang:main Feb 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants