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

GH2440: Add EnvironmentVariable<T> alias #2455

Merged
merged 2 commits into from
Feb 20, 2019
Merged

Conversation

gitfool
Copy link
Contributor

@gitfool gitfool commented Jan 19, 2019

Fixes #2440

Copy link
Member

@devlead devlead left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@devlead
Copy link
Member

devlead commented Feb 20, 2019

@gitfool your changes have been merged, thanks for your contribution 👍

@devlead devlead merged commit d01d3ef into cake-build:develop Feb 20, 2019
@gitfool gitfool deleted the gh2440 branch February 20, 2019 22:57
@gitfool
Copy link
Contributor Author

gitfool commented Feb 22, 2019

@devlead I hit an issue when trying to use Cake after this merge - I've just discovered there's another implementation of this method in Cake.Incubator/EnvironmentExtensions.cs#L64 that causes an error when compiling the build script:

Compiling build script...
(3312,10): error CS0111: Type 'Submission#0' already defines a member called 'EnvironmentVariable' with the same parameter types
Error: Cake.Core.CakeException: Error(s) occurred when compiling build script:
(3312,10): error CS0111: Type 'Submission#0' already defines a member called 'EnvironmentVariable' with the same parameter types
   at Cake.Scripting.Roslyn.RoslynScriptSession.Execute(Script script)
   at Cake.Core.Scripting.ScriptRunner.Run(IScriptHost host, FilePath scriptPath, IDictionary`2 arguments)
   at Cake.Commands.BuildCommand.Execute(CakeOptions options)
   at Cake.CakeApplication.Run(CakeOptions options)
   at Cake.Program.Main()

I don't know how I missed this since I always use Cake.Incubator, but I guess great minds think alike? 😊

Anyway, the question is how to proceed from here? We can either revert, or move the other method over from Cake.Incubator and then remove them from there. Personally, I think it would be nice to keep this in Cake itself without needing another dependency. What do you reckon?

@devlead
Copy link
Member

devlead commented Feb 24, 2019

@gitfool this was bound to happen sooner or later.

Raised issue #2487 to track this.

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.

2 participants