-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Don't include runtime config info during publish #49394
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
Don't include runtime config info during publish #49394
Conversation
|
@dotnet/run-file for reviews |
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
This PR adds a flag to skip including runtime configuration items when publishing, addressing issue #49368.
- Introduce
includeRuntimeConfigInformationparameter toWriteProjectFile - Set the flag based on
BuildTarget != "Publish"inCreateProjectRootElement - Wrap the runtimeconfig
<ItemGroup>generation in a conditional
| targetFilePath: EntryPointFileFullPath, | ||
| artifactsPath: GetArtifactsPath()); | ||
| artifactsPath: GetArtifactsPath(), | ||
| includeRuntimeConfigInformation: BuildTarget != "Publish"); |
Copilot
AI
Jun 13, 2025
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.
Use a case-insensitive comparison when checking BuildTarget to avoid unexpected mismatches. For example: !string.Equals(BuildTarget, "Publish", StringComparison.OrdinalIgnoreCase).
| includeRuntimeConfigInformation: BuildTarget != "Publish"); | |
| includeRuntimeConfigInformation: !string.Equals(BuildTarget, "Publish", StringComparison.OrdinalIgnoreCase)); |
| targetFilePath: EntryPointFileFullPath, | ||
| artifactsPath: GetArtifactsPath()); | ||
| artifactsPath: GetArtifactsPath(), | ||
| includeRuntimeConfigInformation: BuildTarget != "Publish"); |
Copilot
AI
Jun 13, 2025
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] Extract the literal "Publish" into a named constant or enum to eliminate a magic string and improve maintainability.
| includeRuntimeConfigInformation: BuildTarget != "Publish"); | |
| includeRuntimeConfigInformation: BuildTarget != BuildTargets.Publish); |
|
|
||
| var targetDirectory = Path.GetDirectoryName(targetFilePath) ?? ""; | ||
| writer.WriteLine($""" | ||
| if (includeRuntimeConfigInformation) |
Copilot
AI
Jun 13, 2025
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.
Add unit tests covering both paths of includeRuntimeConfigInformation (true and false) to verify the runtime config <ItemGroup> is correctly included or omitted. Use the Fact Skip parameter to reference issue #49368.
Fixes #49368