Include file based program sources#81284
Conversation
There was a problem hiding this comment.
It might be good to add some stronger indicator that these files are not meant to be modified directly. For example, placing in a Package/ or Generated/ subfolder, along with readme in the same or containing dir explaining what is being done in here, as well as perhaps instructions for how to update the sources.
| var root = FindRepoRoot(); | ||
| if (root == null) | ||
| { | ||
| output.WriteLine($"Could not locate repo root. Skipping test. Current directory: {Environment.CurrentDirectory}"); |
There was a problem hiding this comment.
It feels reasonable to fail the test in this case
There was a problem hiding this comment.
It would fail in Helix legs which pick it up but they cannot find the repo root as we don't copy over the whole repo to Helix.
| /// because that would not work in source build (which requires roslyn to build before sdk). | ||
| /// </summary> | ||
| [Fact] | ||
| public void Match() |
There was a problem hiding this comment.
I don't have a strong opposition to doing this in the form of a test, but, it does feel to me more like a script like ./eng/generate-compiler-code.cs.
There was a problem hiding this comment.
That sounds good to me, will try to convert this to a script, thanks.
| Namespace="Microsoft.DotNet.FileBasedPrograms" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <PackageDownload |
There was a problem hiding this comment.
Is this still used? Maybe it ensures that the package is actually restored so that the script can dig it out of the package cache?
There was a problem hiding this comment.
Yes, seemed like best place to put it, I can attach a comment.
|
It would be good to comment at the place the package version is defined, what step to perform to update the in-repo sources. |
Co-authored-by: Rikki Gibson <[email protected]>
I'm not sure that's possible, the Version.Details.props is automatically regenerated by maestro on updates I believe. But perhaps putting a comment at the PackageDownload (where the version is used) is enough? |
See dotnet/dotnet#3244 (comment).
This should unblock our code flow to VMR.