Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Upgrade WebDemo sample to .NET 8 #1059

wants to merge 3 commits into from

Conversation

zhenlan
Copy link
Contributor

@zhenlan zhenlan commented May 21, 2025

  1. Updated the project from .NET 6 to .NET 8
  2. Replaced connection string authentication with DefaultAzureCredential for modern and secure Azure authentication
  3. Updated NuGet packages to their latest stable versions

Credit to GitHub Copilot agent :)

@zhenlan zhenlan requested a review from Copilot May 21, 2025 22:34
Copy link
Contributor

@Copilot Copilot AI left a 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 with DefaultAzureCredential and dynamic refresh
  • Renames namespaces/files from WebDemoNet6 to WebDemo 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");
Copy link
Preview

Copilot AI May 21, 2025

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.

Suggested change
Environment.GetEnvironmentVariable("AppConfigEndpoint");
Environment.GetEnvironmentVariable("AppConfig__Endpoint");

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant