-
Notifications
You must be signed in to change notification settings - Fork 415
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
Adding ability to select the .NET language on create #2667
Changes from all commits
a50430e
7646f55
b75b742
1c489f1
95d729d
c18b9d3
baef606
7ef25ec
3e80f07
82ba7ec
b43257c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ public static class WorkerRuntimeLanguageHelper | |
private static readonly IDictionary<WorkerRuntime, IEnumerable<string>> availableWorkersRuntime = new Dictionary<WorkerRuntime, IEnumerable<string>> | ||
{ | ||
{ WorkerRuntime.dotnet, new [] { "c#", "csharp", "f#", "fsharp" } }, | ||
{ WorkerRuntime.dotnetIsolated, new [] { "dotnet-isolated" } }, | ||
{ WorkerRuntime.dotnetIsolated, new [] { "dotnet-isolated", "c#-isolated", "csharp-isolated", "f#-isolated", "fsharp-isolated" } }, | ||
{ WorkerRuntime.node, new [] { "js", "javascript", "typescript", "ts" } }, | ||
{ WorkerRuntime.python, new [] { "py" } }, | ||
{ WorkerRuntime.java, new string[] { } }, | ||
|
@@ -41,7 +41,7 @@ public static class WorkerRuntimeLanguageHelper | |
private static readonly IDictionary<WorkerRuntime, string> workerToDefaultLanguageMap = new Dictionary<WorkerRuntime, string> | ||
{ | ||
{ WorkerRuntime.dotnet, Constants.Languages.CSharp }, | ||
{ WorkerRuntime.dotnetIsolated, Constants.Languages.CSharp }, | ||
{ WorkerRuntime.dotnetIsolated, Constants.Languages.CSharpIsolated }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need entries for F#? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see it's the defaults There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, figured that the default of C# makes the most sense |
||
{ WorkerRuntime.node, Constants.Languages.JavaScript }, | ||
{ WorkerRuntime.python, Constants.Languages.Python }, | ||
{ WorkerRuntime.powershell, Constants.Languages.Powershell }, | ||
|
@@ -55,7 +55,8 @@ public static class WorkerRuntimeLanguageHelper | |
{ Constants.Languages.TypeScript, new [] { "ts" } }, | ||
{ Constants.Languages.Python, new [] { "py" } }, | ||
{ Constants.Languages.Powershell, new [] { "pwsh" } }, | ||
{ Constants.Languages.CSharp, new [] { "csharp", "dotnet", "dotnet-isolated", "dotnetIsolated" } }, | ||
{ Constants.Languages.CSharp, new [] { "csharp", "dotnet" } }, | ||
{ Constants.Languages.CSharpIsolated, new [] { "dotnet-isolated", "dotnetIsolated" } }, | ||
{ Constants.Languages.Java, new string[] { } }, | ||
{ Constants.Languages.Custom, new string[] { } } | ||
}; | ||
|
@@ -67,7 +68,9 @@ public static class WorkerRuntimeLanguageHelper | |
|
||
public static readonly IDictionary<WorkerRuntime, IEnumerable<string>> WorkerToSupportedLanguages = new Dictionary<WorkerRuntime, IEnumerable<string>> | ||
{ | ||
{ WorkerRuntime.node, new [] { Constants.Languages.JavaScript, Constants.Languages.TypeScript } } | ||
{ WorkerRuntime.node, new [] { Constants.Languages.JavaScript, Constants.Languages.TypeScript } }, | ||
{ WorkerRuntime.dotnet, new [] { Constants.Languages.CSharp, Constants.Languages.FSharp } }, | ||
{ WorkerRuntime.dotnetIsolated, new [] { Constants.Languages.CSharpIsolated, Constants.Languages.FSharpIsolated } } | ||
}; | ||
|
||
public static string AvailableWorkersRuntimeString => | ||
|
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.
Do we still need
dotnet-isolated
in the array containingc#-isolated
andcsharp-isolated
here?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.
I'll have to have a second look at it, but I think the reason I kept
dotnet-isolated
in there was for backwards compatibility, as prior to this change that's how things were identified.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.
I've had a bit of a poke around and I don't think it can be removed with a lot of other cascading changes.
Removing
dotnet-isolated
will cause existing projects to fail to launch usingfunc start
as it's unable to detect the worker type. I also tested changing the worker type inlocal.settings.json
toc#-isolated
but that then errors within the WebJobs dependency as it couldn't find the functions languagec#-isolated
, so there'd be a downstream impact.Additionally, all templates would need to be updated to either say
c#-isolated
orf#-isolated
in them before the change would usable.Ultimately, the additional worker types are probably not doing anything of value at present, they are more future proofing 😅