feat(dotnet): Support for JSII_DEBUG and JSII_RUNTIME #724
Conversation
|
The title of this Pull Request does not conform with Conventional Commits guidelines. It |
assyadh
changed the title
| RedirectStandardInput = true, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true | ||
| } | ||
| }; | ||
|
|
||
| _process.StartInfo.EnvironmentVariables.Add("JSII_AGENT", "DotNet/" + Environment.Version); | ||
| _process.StartInfo.EnvironmentVariables.Add(JsiiAgent, "DotNet/" + Environment.Version); |
There was a problem hiding this comment.
Any chance we can be more "accurate" than "DotNet"? Is it possible to get ideas of the framework build version or something?
There was a problem hiding this comment.
Yup, we do it in the SDK, will do the same here.
packages/jsii-dotnet-runtime/src/Amazon.JSII.Runtime/Services/NodeProcess.cs
Outdated
Show resolved
Hide resolved
| @@ -915,7 +915,7 @@ public void OptionalAndVariadicArgumentsTest() | |||
| [Fact(DisplayName = Prefix + nameof(JsiiAgent))] | |||
| public void JsiiAgent() | |||
| { | |||
| Assert.Equal("DotNet/" + Environment.Version.ToString(), JsiiAgent_.JsiiAgent); | |||
| Assert.Equal("DotNet/" + Environment.Version.ToString() + "/.NETStandard,Version=v2.0/1.0.0.0", JsiiAgent_.JsiiAgent); | |||
There was a problem hiding this comment.
What does it take for this to break? Does it need manual action (source change somewhere else)? Or can it change just by getting updated toolkit?
There was a problem hiding this comment.
An update to the framework that your app is targeting.
There is no 'clean' way to get the running .NET Core version:
I could just test that it StartsWith("DotNet")?
There was a problem hiding this comment.
As an aside, were we not going to change to core 2.1 target?
Standard is great, except it makes it look like we support framework - and I don't even really want to give developers that option.
| @@ -915,7 +915,7 @@ public void OptionalAndVariadicArgumentsTest() | |||
| [Fact(DisplayName = Prefix + nameof(JsiiAgent))] | |||
| public void JsiiAgent() | |||
| { | |||
| Assert.Equal("DotNet/" + Environment.Version.ToString(), JsiiAgent_.JsiiAgent); | |||
| Assert.Equal("DotNet/" + Environment.Version.ToString() + "/.NETStandard,Version=v2.0/1.0.0.0", JsiiAgent_.JsiiAgent); | |||
There was a problem hiding this comment.
As an aside, were we not going to change to core 2.1 target?
Standard is great, except it makes it look like we support framework - and I don't even really want to give developers that option.
| var assemblyFileVersionAttribute = assembly.GetCustomAttribute(typeof(AssemblyFileVersionAttribute)) as AssemblyFileVersionAttribute; | ||
| var frameworkAttribute = assembly.GetCustomAttribute(typeof(TargetFrameworkAttribute)) as TargetFrameworkAttribute; | ||
| return new Tuple<string, string>( | ||
| frameworkAttribute == null ? "Unknown" : frameworkAttribute.FrameworkName, |
There was a problem hiding this comment.
frameworkAttribute?.FrameworkName ?? "Unknown"
:P This is just a nit.
|
Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it! |
Fixes #438
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.