-
Notifications
You must be signed in to change notification settings - Fork 73
Upgrade WebDemo sample to .NET 8 #1059
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
Upgrades the WebDemo sample from .NET 6 to .NET 8, replaces connection-string auth with DefaultAzureCredential
, and updates NuGet packages to their latest stable versions.
- Bumps target framework to net8.0 and adds
Azure.Identity
support - Refactors
Program.cs
to use Azure App Configuration withDefaultAzureCredential
and dynamic refresh - Renames namespaces/files from
WebDemoNet6
toWebDemo
and updates solution/project definitions
Reviewed Changes
Copilot reviewed 80 out of 80 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
examples/DotNetCore/WebDemo/WebDemo/WebDemo.csproj | Updated target framework to net8.0 and bumped package versions |
examples/DotNetCore/WebDemo/WebDemo/Settings.cs | Renamed namespace to WebDemo |
examples/DotNetCore/WebDemo/WebDemo/Program.cs | Integrated Azure App Configuration using DefaultAzureCredential and dynamic refresh |
examples/DotNetCore/WebDemo/WebDemo/Pages/_ViewImports.cshtml | Updated Razor imports namespace |
examples/DotNetCore/WebDemo/WebDemo/Pages/Shared/_Layout.cshtml | Corrected stylesheet reference and updated footer text |
examples/DotNetCore/WebDemo/WebDemo/Pages/*.cshtml.cs | Updated page model namespaces |
examples/DotNetCore/WebDemo/WebDemo.sln | Renamed solution/project entries and updated VS version & GUIDs |
Comments suppressed due to low confidence (3)
examples/DotNetCore/WebDemo/WebDemo/Program.cs:19
- The closing quote appears to be a curly double-quote which will cause a compile error; replace it with a straight
"
.
.Select("WebDemo:*”)
examples/DotNetCore/WebDemo/WebDemo/Program.cs:23
- [nitpick] There are no tests validating that dynamic configuration refresh works as expected; consider adding integration tests to cover the
AddAzureAppConfiguration
refresh behavior.
refreshOptions.RegisterAll();
examples/DotNetCore/WebDemo/WebDemo/Pages/Shared/_Layout.cshtml:49
- [nitpick] The privacy link was removed in this footer; re-adding a link to the privacy page helps maintain consistent navigation and accessibility.
© WebDemo for Azure App Configuration
|
||
// Get the Azure App Configuration endpoint from environment or settings | ||
string? appConfigEndpoint = builder.Configuration["AppConfig:Endpoint"] ?? | ||
Environment.GetEnvironmentVariable("AppConfigEndpoint"); |
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.
[nitpick] Consider using the ASP.NET Core environment variable naming convention (AppConfig__Endpoint
) so the configuration binder can automatically map nested settings.
Environment.GetEnvironmentVariable("AppConfigEndpoint"); | |
Environment.GetEnvironmentVariable("AppConfig__Endpoint"); |
Copilot uses AI. Check for mistakes.
Credit to GitHub Copilot agent :)