-
Notifications
You must be signed in to change notification settings - Fork 50
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
Update Package.swift with Kitura-net 1.6 #14
Conversation
great! Did we decide if we want to only specify majorVersion and not majorVersion + minor? So that it could potentially work with more Kitura versions? |
Using only majorVersion would be best for us and other consumers to avoid dependency conflicts. For the PR I figured I'd try to be consistent with the current patterns in Package.swift. I'd much prefer to see just majorVersion going forward, for what it's worth. |
We would also prefer just majorVersion; we found that this broke us for a few days when our dependencies updated a common dependency with major + minor versioning (for us it was KituraNet moving to LoggerAPI 1.6 whilst HeliumLogger stayed with 1.5). I guess this would have to be a change for most of the dependency tree in order to truly work well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change breaks the dependency graph: Kitura-net 1.6 conflicts with Kitura-Request 0.6, which has a dependency on Kitura-net 1.5. Please double-check your change and make sure it builds cleanly before resubmitting.
Thinking about it, if Kitura-Request depends on Kitura-net then we don't need the duplicate dependency in our Package.swift - I think the best thing to do here is to remove the Kitura-net dependency from Package.swift and bump Kitura-Request to 0.7. I'm happy to do this or you can - let me know what you want to do. |
That sounds good. I'll get an updated PR to you soon. |
Ah yeah, if Kitura-Request brings in Kitura-Net automatically- perhaps removing the dependency would work. OR just make Kitura-Net only specified by majorVersion, and omit minor- as discussed earlier. |
@mattcolegate check out last commit-- this builds for me @rfdickerson BTW, I get dependency errors when trying to use just the majorVersion in Kitura-Request. |
@jkingoliver Builds for me too, so change approved! @rfdickerson, I built with the changes (so 4 dependencies) all at majorVersion only and it went through cleanly, but in order to prevent dependency errors in the future I want to hold off delivering that until all our dependencies are majorVersion only. I've raised issue #16 to track this. |
No description provided.