-
Notifications
You must be signed in to change notification settings - Fork 103
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
Change package to com.github.sbt.git
?
#216
Comments
Ah right, I forgot about the sbt factor. If the keys don't change names but their packages do, I assume sbt may error if they are both loaded into a build? But I still think this is better than silent classpath non-determinism when the keys and package names are unchanged. |
Yeah - so if that turns out to be a problem, perhaps it would be better to not change the package, and release a 'transitional' 1.x artifact that doesn't have any artifacts of its own but has a transitive dependency on the artifact with the new groupId? |
Hmmm. That seems like a good idea, but I'm confused about execution. It seems like the new artifact under the new group id should depend on the empty, transitional old-group-id artifact, to force the eviction. |
To prevent classpath problems when it is on the same classpath with the sbt-git when it was published with the old Refs sbt#216
Tested (#217), indeed that won't work
Hmm, yeah, both have their advantages. I don't have a strong preference. |
Thanks for investigating. I think any of these options is better than non-determinism. Your call, I'll be happy to see this unblocked :) |
I'm fine with #218. |
As discussed in #215: if we release sbt-pgp under the new groupId (#205), due to transitive plugin dependencies people could get both the old and the new version on their plugin classpath, leading to undeterministic behavior.
We could avoid that problem by also changing the package name to something like
com.github.sbt.git
. That solves the classpath problem, but since sbt-git is an autoplugin it might confuse sbt? Or would that be fine? Perhaps we should test this first?The text was updated successfully, but these errors were encountered: