cake-contrib / Cake.FluentMigrator

FluentMigrator task for Cake
https://cakebuild.net/extensions/cake-fluentmigrator/
MIT License
6 stars 14 forks source link

Add explicit Nuget reference to Cake.Core #15

Closed carlos-vicente closed 5 years ago

carlos-vicente commented 6 years ago

Add explicit Nuget reference to Cake.Core in package. image As the above image shows, there is a reference to assembly Cake.Core version 0.17.0, but the nuget package does not depend on the Cake.Core nuget.

devlead commented 6 years ago

Cake.Core ships with the Cake runner and should be an private reference only needed to build the addin.

carlos-vicente commented 6 years ago

I'm not arguing against that, it makes sense, but that hides a version requirement. We have a project using Cake with Cake.FluentMigrator and I wanted to bump the cake version because of something new that was introduced in a later version (later than 0.17.0).

Looking at the Cake.FluentMigrator package dependencies, there is nothing saying that it needs version 0.17.0, so when running the cake script with any version higher than 0.17.0, it blows up when compiling because Cake.FluentMigrator has a reference to Cake.Core 0.17.0.

It is just a matter of managing expectations, do you agree?

Best regards

devlead commented 6 years ago

Well it needs 0.17.0 or newer, so it worked all versions from it up till 0.22.0 when breaking changes was introduced. If it was an explicit dependency to Cake.Core then that version would be downloaded even though Cake already ships with Cake.Core and potentially cause issues.

In future versions of Cake nuget dependencies will be handled and potentially it could be added as an dependency, but at a moment it'll cause issues, which is why it shouldn't be added as an explicit packages dependency.

carlos-vicente commented 6 years ago

So if I reference version 0.22.0 it should work? I've only tested with the latest version 0.22.2

devlead commented 6 years ago

Yes, last breaking change in Cake.Core was 0.22.0

carlos-vicente commented 6 years ago

It worked, but I keep my statement that every extension package should have an package reference, stating the minimum reference to the Cake main package (or Cake.Core package).

Looking at multiple packages that don't have this reference, I don't know which is the minimum version the extension will work on. I either have poke around the assemblies trying to figure it out (which I had to do...) or I just install the latest version and cross my fingers in the hopes it will work.

Take this just as a constructive criticism, feel free to close the issue if you don't want to do this.

PS: some cake extension packages have the nuget package version (https://www.nuget.org/packages/Cake.FileHelpers/1.0.4.16), others don't (like this one).

viacheslave commented 6 years ago

As Cake moved ahead (0.23), there is still no way get the extension to work as it says it requires Cake.Core 0.17:

Warning: The assembly 'Cake.FluentMigrator, Version=0.3.0.0, Culture=neutral, PublicKeyToken=null'
is referencing an older version of Cake.Core (0.17.0).
This assembly need to reference at least Cake.Core version 0.22.0.
Another option is to downgrade Cake to an earlier version.

Options:

Please reconsider the dependencies.

dealproc commented 6 years ago

Interested in seeing if the PR from @viacheslave is accepted or not, as I'd like to use this with a dotnet core project I'm developing right now.

Ninglin commented 6 years ago

I'm also interested in seeing that PR reach master. Let's wait and see.

dealproc commented 6 years ago

for now, i've scripted it into my cake script instead of depending on the addin... just needed to get past this. cool setup tho.

gep13 commented 5 years ago

I am away to update this addin use the latest version of Cake within the compilation of the addin. This should hopefully resolve most issues discussed here. I am not planning on adding a dependency on Cake.Core in the nuspec file though, as I still don't believe that this is required.

Please let me know if there are any follow up comments about this.