-
Notifications
You must be signed in to change notification settings - Fork 9
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
GOV-29: Fixing NuGet publishing failing due to GitHub runners now having NuGet.org registered as a package source #324
Conversation
…t.org registered as a package source
WalkthroughThe changes revolve around improving the NuGet package publishing process. Specifically, they introduce a check to prevent the duplication of NuGet.org package sources during the setup phase and refine the action's versioning strategy by specifying a particular version tag for the publishing action. This enhances the reliability and specificity of the workflow, ensuring that the setup is more precise and that the action used for publishing is explicitly defined. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
You might need this @Psichorex. |
# Register NuGet.org as a package source since GitHub runners don't necessarily have it by default. | ||
$existingSource = Get-PackageSource -Name NuGet.org -ErrorAction SilentlyContinue | ||
|
||
if (-not $existingSource) | ||
{ | ||
Register-PackageSource -Name NuGet.org -Location https://api.nuget.org/v3/index.json -ProviderName NuGet | ||
} | ||
else | ||
{ | ||
Write-Output 'Package source for NuGet.org is already registered.' | ||
} | ||
|
||
# Get the latest version of the package. | ||
$latestPackage = Find-Package -Name Microsoft.SourceLink.GitHub -Source NuGet.org | Sort-Object Version -Descending | Select-Object -First 1 |
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.
📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [12-57]
The script efficiently finds the latest version of the Microsoft.SourceLink.GitHub
package and handles both solution and project files appropriately. The use of Get-ChildItem
with -Recurse
and filtering for solution and project files, followed by conditional logic to handle different scenarios, is well-implemented. The XML manipulation to add the PackageReference
is correctly done using PowerShell's XML capabilities. However, consider adding error handling around external calls like dotnet sln list
and XML file modifications to ensure the script gracefully handles any failures.
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.
If any of these fail then we need the whole workflow fail, as it happens currently.
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.
@Piedone, understood. Ensuring that the workflow fails on any error in the script is a robust approach to catch issues early and maintain the integrity of the publishing process. It's clear that the current script design intentionally lacks explicit error handling for this reason. Documenting this behavior within the script or the workflow documentation could be beneficial for clarity and to inform future contributors of this intentional design choice.
GOV-29