Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3781 +/- ##
==========================================
- Coverage 81.27% 81.23% -0.04%
==========================================
Files 622 622
Lines 39322 39307 -15
Branches 6415 6375 -40
==========================================
- Hits 31958 31933 -25
- Misses 6366 6387 +21
+ Partials 998 987 -11 ☔ View full report in Codecov by Sentry. |
marksvc
left a comment
There was a problem hiding this comment.
@marksvc reviewed 53 files and all commit messages, and made 3 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on pmachapman).
src/SIL.XForge.Scripture/Program.cs line 32 at r5 (raw file):
Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Production";
I've learned that it can be safer to fall back to the Development environment settings. Then we need to specifically request to use the Production ports, paths, remote URIs, DBs, etc. And if we don't specify, then we more safely write to the less important addresses. What do you think about that here?
src/SIL.XForge.Scripture/Program.cs line 44 at r5 (raw file):
} config.AddEnvironmentVariables();
I think at least in Linux one would expect
- the config file settings to be overridden by
- the environment settings, which are overridden by
- the commandline arguments.
Is there a reason the order in the code above appears to be
- commandline, overridden by
- config files, overridden by
- environment variables.
?
I wouldn't want the arguments to dotnet run to be overridden by the config files.
Oh, I see that Devin discussed this some. Yeah, so if we put the config.AddCommandLine after config.AddEnvironmentVariables that may allow commandline arguments to override the others.
Hmm, Devin says it's rare to specify args, but I do on my workstation :). I specify some --node-options. (Though I think recently that has actually been overridden and not working. But ideally it would be working :)
src/SIL.XForge.Scripture/Startup.cs line 100 at r5 (raw file):
private readonly HashSet<string> SpaPostRoutes = []; public Startup(IConfiguration configuration, IWebHostEnvironment env)
Devin pointed out that with Startup.cs changes, PackageReference Hangfire.Autofac can be removed from SIL.XForge.csproj.
This PR updates the projects, devcontainers, dependencies, and docker containers to .NET 10.
This change is