Conversation
ptr727
commented
Apr 14, 2026
- Updated build-library-task.yml to specify OutputPath and PackageOutputPath for dotnet build.
- Changed concurrency group in run-periodic-codegen-pull-request.yml to target main/codegen.
- Modified launch.json to reflect new output paths for the LanguageTagsCreate project.
- Enhanced AGENTS.md with project configuration guidelines and centralized NuGet package versions.
- Updated LanguageData/rfc5646 and LanguageData/rfc5646.json with new language subtags and descriptions.
- Cleaned up GlobalUsings.cs files by removing unused namespaces.
- Adjusted LanguageTags.csproj and LanguageTagsCreate.csproj to centralize package versions and remove unnecessary properties.
- Improved HttpClientFactory.cs with resilience strategies using Polly for better error handling.
- Added Directory.Build.props and Directory.Packages.props for centralized project settings and package version management.
- Updated LanguageTagsTests project to remove unnecessary properties and centralize package references.
There was a problem hiding this comment.
Pull request overview
Refactors solution-wide build/package configuration (central MSBuild defaults + central NuGet versions), updates the RFC 5646 registry data, and adjusts CI/codegen workflows and the codegen tool’s HTTP resilience behavior.
Changes:
- Centralized MSBuild defaults and NuGet package versions via
Directory.Build.propsandDirectory.Packages.props, and simplified individual project files accordingly. - Updated codegen automation/workflows (concurrency, permissions, PR creation/triggering) and developer tooling configuration.
- Refreshed RFC 5646 language subtag registry sources and regenerated the corresponding C# data.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Directory.Build.props | Adds repo-wide MSBuild defaults (TFM, analyzers, warnings-as-errors, central package mgmt, artifacts path). |
Directory.Packages.props | Introduces central NuGet package version management. |
LanguageTags/LanguageTags.csproj | Removes duplicated analyzer/framework settings; switches to centrally-managed package versions; explicitly marks library packable. |
LanguageTagsCreate/LanguageTagsCreate.csproj | Removes duplicated analyzer/framework settings; switches to centrally-managed package versions. |
LanguageTagsTests/LanguageTagsTests.csproj | Removes duplicated analyzer/framework settings; switches to centrally-managed package versions. |
LanguageTagsCreate/HttpClientFactory.cs | Expands Polly resilience with retry/circuit breaker logging and shared handler configuration tweaks. |
LanguageTags*/GlobalUsings.cs (all) | Removes unused global using directives. |
LanguageTags/LogOptions.cs | Removes an unused using. |
LanguageData/rfc5646 | Updates raw IANA registry snapshot (new/updated subtags, metadata). |
LanguageData/rfc5646.json | Updates JSON-converted registry snapshot (aligned with raw update). |
LanguageTags/Rfc5646DataGen.cs | Regenerates embedded RFC 5646 dataset C# code with updated records/date. |
.github/workflows/build-library-task.yml | Builds using explicit OutputPath/PackageOutputPath rather than --output. |
.github/workflows/run-periodic-codegen-pull-request.yml | Adjusts concurrency grouping for the periodic codegen workflow. |
.github/workflows/run-codegen-pull-request-task.yml | Uses GITHUB_TOKEN for PR creation; adds permissions; closes/reopens PR via PAT to trigger workflows. |
.github/workflows/merge-bot-pull-request.yml | Updates merge conditions to align with codegen PRs authored by github-actions[bot]. |
.vscode/launch.json | Updates debug target paths to .artifacts output. |
AGENTS.md | Documents new centralized project configuration conventions. |
.github/copilot-instructions.md | Updates documentation formatting and adds centralized project configuration guidance. |
| "preLaunchTask": ".Net Build", | ||
| "program": "${workspaceFolder}/LanguageTagsCreate/bin/Debug/net10.0/LanguageTagsCreate.dll", | ||
| "program": "${workspaceFolder}/.artifacts/bin/LanguageTagsCreate/debug/LanguageTagsCreate.dll", | ||
| "args": [ | ||
| "--codepath", | ||
| "${workspaceFolder}", | ||
| ], | ||
| "cwd": "${workspaceFolder}/LanguageTagsCreate/bin/Debug/net10.0", | ||
| "cwd": "${workspaceFolder}/.artifacts/bin/LanguageTagsCreate/debug", | ||
| "console": "internalConsole", |
There was a problem hiding this comment.
preLaunchTask runs .Net Build, which (per .vscode/tasks.json) just executes dotnet build with default output paths. This launch config now points program/cwd at /.artifacts/..., so debugging will fail unless the build is also configured to emit binaries there. Either update the build task/MSBuild props to output into .artifacts (e.g., enable the SDK artifacts output feature / set OutputPath) or revert program/cwd to the standard bin/Debug/<tfm> location.
| <AnalysisLevel>latest-all</AnalysisLevel> | ||
| <AnalysisMode>All</AnalysisMode> | ||
| <EnableNETAnalyzers>true</EnableNETAnalyzers> | ||
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> |
There was a problem hiding this comment.
ArtifactsPath is defined here, but nothing enables the SDK's artifacts output layout, so local builds will still emit to bin//obj/ by default. Since .vscode/launch.json was updated to reference /.artifacts/bin/..., consider explicitly enabling artifacts output (or setting OutputPath/BaseOutputPath consistently) so the .artifacts path actually exists after a normal dotnet build.
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> | |
| <TreatWarningsAsErrors>true</TreatWarningsAsErrors> | |
| <UseArtifactsOutput>true</UseArtifactsOutput> |
| private static bool IsTransientFailure(Outcome<HttpResponseMessage> outcome) => | ||
| outcome.Exception is not null | ||
| ? outcome.Exception is not (OperationCanceledException or BrokenCircuitException) | ||
| : outcome.Result is not null && (int)outcome.Result.StatusCode is 408 or 429 or >= 500; | ||
There was a problem hiding this comment.
IsTransientFailure treats all OperationCanceledException as non-transient. This also includes TaskCanceledException raised by HttpClient.Timeout, so timeouts won’t be retried and won’t contribute to circuit-breaker metrics, reducing resilience for slow/transient connections. Consider distinguishing user-requested cancellation (don’t retry when the caller token is canceled) from timeouts (retry), e.g., by using the strategy callback args to check args.Context.CancellationToken.IsCancellationRequested or by adding a Polly timeout strategy and handling that exception type explicitly.
| privatestaticboolIsTransientFailure(Outcome<HttpResponseMessage>outcome)=> | |
| outcome.Exceptionis not null | |
| ?outcome.Exceptionis not (OperationCanceledException or BrokenCircuitException) | |
| :outcome.Resultis not null&&(int)outcome.Result.StatusCodeis408 or 429 or >=500; | |
| privatestaticboolIsTransientFailure(Outcome<HttpResponseMessage>outcome) | |
| { | |
| if(outcome.ExceptionisBrokenCircuitException) | |
| { | |
| returnfalse; | |
| } | |
| if(outcome.ExceptionisOperationCanceledExceptionoperationCanceledException) | |
| { | |
| return!IsUserCancellation(operationCanceledException); | |
| } | |
| if(outcome.Exceptionis not null) | |
| { | |
| returntrue; | |
| } | |
| returnoutcome.Resultis not null&&(int)outcome.Result.StatusCodeis408 or 429 or >=500; | |
| } | |
| privatestaticboolIsUserCancellation(OperationCanceledExceptionexception)=> | |
| exception.CancellationToken.CanBeCanceled&&exception.CancellationToken.IsCancellationRequested; |