Started using Roslyn 3.0.0-beta (scripting) #78
Conversation
| <configuration> | ||
| <packageSources> | ||
| <add key="nuget.org" value="https://api.nuget.org/v3/index.json" protocolVersion="3" /> | ||
| <add key="Roslyn nightly builds" value="https://dotnet.myget.org/F/roslyn/api/v3/index.json" /> |
There was a problem hiding this comment.
we should not be using the nightly build feed. I'd rather internalize the DLLs so that we can publish a nuget package
There was a problem hiding this comment.
By internalizing you mean that we should keep the Roslyn dll's in the repo?
There was a problem hiding this comment.
Regardless how to distribute, we still ned to bring in the 3.0.0 dll's from somewhere before we create the NuGet package?
There was a problem hiding this comment.
yeah in scriptcs I just copied the DLLs into the lib folder and included them into the created nuget package scriptcs/scriptcs@44f666e
Alternatively, but I'd like to avoid it, we can take Roslyn 3 from myget and publish on nuget under a different name and reference that 😎
There was a problem hiding this comment.
Going for the lib folder approach. Do you know what we have to do in csproj to include these files in the NuGet package?
There was a problem hiding this comment.
something like this should work
<ItemGroup>
<Content Include="$(OutputPath)\Microsoft.CodeAnalysis.CSharp.Scripting.dll">
<PackagePath>lib/netstandard1.6/</PackagePath>
<Pack>true</Pack>
</Content>
</ItemGroup>
src/Dotnet.Script/Program.cs
Outdated
| @@ -81,16 +81,16 @@ private static void RunScript(string file, string config, bool debugMode, IEnume | |||
| } | |||
|
|
|||
| var directory = Path.IsPathRooted(file) ? Path.GetDirectoryName(file) : Path.GetDirectoryName(Path.Combine(Directory.GetCurrentDirectory(), file)); | |||
| var sourceText = SourceText.From(new FileStream(file, FileMode.Open)); | |||
| var context = new ScriptContext(sourceText, directory, config, args, file); | |||
| var sourceText = SourceText.From(new FileStream(file, FileMode.Open),Encoding.UTF8); | |||
There was a problem hiding this comment.
you should not hardcode the encoding. SourceText.From(stream) will internally try to detect encoding using BOM and if it's not found, it will fallback to UTF8 automatically
|
|
||
| if (!string.IsNullOrWhiteSpace(context.FilePath)) | ||
| { | ||
| opts = opts.WithFilePath(context.FilePath); | ||
| opts = opts.WithFilePath(context.FilePath).WithFileEncoding(Encoding.UTF8); |
There was a problem hiding this comment.
same here, encoding will not be needed if you change here https://github.com/filipw/dotnet-script/blob/master/src/Dotnet.Script.Core/ScriptCompiler.cs#L108 to use the new stream-based API https://github.com/dotnet/roslyn/blob/master/src/Scripting/CSharp/CSharpScript.cs#L44
|
thanks a lot! just have a few comments |
| @@ -11,10 +11,26 @@ | |||
| <GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute> | |||
| <GenerateAssemblyCompanyAttribute>false</GenerateAssemblyCompanyAttribute> | |||
| <GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute> | |||
| <Version>0.2.0</Version> | |||
There was a problem hiding this comment.
this is not needed, there is already a <VersionPrefix />
| @@ -18,12 +18,35 @@ | |||
| <GenerateAssemblyConfigurationAttribute>false</GenerateAssemblyConfigurationAttribute> | |||
| <GenerateAssemblyCompanyAttribute>false</GenerateAssemblyCompanyAttribute> | |||
| <GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute> | |||
| <Version>0.11.0</Version> | |||
There was a problem hiding this comment.
this is not needed, there is already a <VersionPrefix />
There was a problem hiding this comment.
The version properties are confusing.
Related:
There was a problem hiding this comment.
@seesharper sure, I see you've fixed it. Mine was just a tangential comment.
|
looks good, can you please just fix the version and we merge it |
|
thanks a lot |
Removed the DebugScriptRunner.
Think we still need to specify UTF8