Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added: command plugin to download model files. #79

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Arpit160399
Copy link

This pull request addresses issue #62.

Changes Made:

  • Introduced a version-specific Swift package file to facilitate plugin capability for Swift toolchain 5.9 and above.
  • Added a new command plugin within the service to enhance functionality.

Additional Notes:

I attempted to implement gatekeeping using availability, but network permissions need to be set at the package level, which is only available from Swift toolchain 5.9 onward.

@Arpit160399 Arpit160399 marked this pull request as draft May 3, 2024 09:22
@Arpit160399 Arpit160399 marked this pull request as ready for review May 3, 2024 09:23
Copy link
Member

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

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

I've added some comments

Plugins/SotoCodeModelDownloaderPlugin/plugin.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/plugin.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
Plugins/SotoCodeModelDownloaderPlugin/Downloader.swift Outdated Show resolved Hide resolved
@@ -0,0 +1,60 @@
// swift-tools-version: 5.9
import PackageDescription
Copy link
Member

Choose a reason for hiding this comment

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

In Soto (as with SwiftNIO) we look to support the last three versions of Swift. So for the moment that'd be 5.8, 5.9 and 5.10. Currently with your setup the download plugin is only available for 5.9, not 5.10. I would instead create a Package.@swift-5.8.swift which is a copy of the current Package.swift and make this version the default Package.swift

Copy link
Author

Choose a reason for hiding this comment

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

So, with the current package setup, if the current Swift version doesn't match any version-specific manifest, the package manager will pick the manifest with the most compatible tools version. This means the package manager will pick Package.swift for Swift 5.8 and Package@swift-5.9.swift for Swift 5.9 and above, as its tools version will be most compatible with future versions of the package manager. I read this in the documentation here.

In terms of readability and maintainability, I agree with your approach of keeping the 5.8 version tag instead of 5.9.

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.

None yet

2 participants