Removed DotNetCoreTestExtensions.cs.

pbrindley commented 1 year ago

This class contained extension methods for methods in Cake.Common that were removed. This could cause problems when importing the Cake.Incubator addin.

See issue #115

Description

Removed extension methods for now removed methods from a dependency package.

Motivation and Context

Upgrading from an older version of Cake, we also had to update Cake.Incubator, however I kept seeing an error loading Cake.Common. I found the above issue related to references to methods in Cake.Common that had been removed. I created this change to remove this error.

How Has This Been Tested?

I have ran the unit tests in Visual Studio and those not marked as skip have passed.

I have referenced the built dll in our build script and it loads successfully and the script runs as expected. Both on my local machine and in our UAT environment (Team City build was successful).

Screenshots (if appropriate):

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[x] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[ ] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

gitfool commented 1 year ago

@nils-a This needs to be merged now that Cake 3.0 has been released.

nils-a commented 1 year ago

@gitfool, @pbrindley, I feel those were renamed from DotNetCore* to DotNet* - shouldn't the extensions be renamed as well? Or are they really no longer needed?

gitfool commented 1 year ago

Yes they were renamed with the previous names remaining until they were finally removed in Cake 3.0. To be honest, I don't know if they should be renamed or removed from this extension. I merely found this pull request and saw that it would fix the issue.

pbrindley commented 1 year ago

Hello, sorry I didn't reply before but I didn't get a notification for the comments. From what I read here https://github.com/cake-build/cake/pull/2837#issuecomment-688275062 I took it be that the methods had been removed completely from Cake.Common rather than having been renamed, so I went ahead and removed the extension methods.

I'm going through our solutions at work and moving to up to date versions of Cake (so we don't have to have Build Tools 2017 installed anymore, yes we are a little out of date!). Cake.Incubator referencing those methods was causing the load for Cake.Common to fail, because it was missing methods.

If I'm wrong in this and the methods have actually just been renamed then I can remove the pull request.

nils-a commented 1 year ago

@pbrindley, https://github.com/cake-build/cake/pull/2837#issuecomment-688275062 was something that went into Cake 1.0.0. I doubt that it still is a problem. (I should have closed the issue here, though.)

However, DotNetCoreTest was marked obsolete in version 2.0.0 (https://github.com/cake-build/cake/pull/3623) and removed in version 3.0.0 (https://github.com/cake-build/cake/pull/3992)

So, if you're up to it, instead of removing DotNetCoreTestExtensions it would be better if you renamed all DotNetCore* to DotNet*. That should also take care of your errors.

pbrindley commented 1 year ago

@nils-a I'll make that change and make a new create a new merge.

cake-contrib / Cake.Incubator