Skip to content
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

Rename SdkPackageUpdater => PackageReferenceUpdater, for clarity #10788

Conversation

rhyskoedijk
Copy link
Contributor

@rhyskoedijk rhyskoedijk commented Oct 15, 2024

What are you trying to accomplish?

I'm looking at how #10260 can be fixed, but the current terminology is a little confusing; It would be good to clear this up.

The change adds documentation to clarify the scenarios where a "packages.config" update and "PackageReference" update could happen. The current terminology, "SdkPackageUpdater", gives the impression that "PackageReference" will only appear in SDK-style project files, which is not correct (see code comments).

Anything you want to highlight for special attention from reviewers?

No changes to behaviour, purely documentation.

How will you know you've accomplished your goal?

I can easily identify which updater class is applicable to each project file format.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Oct 15, 2024
@rhyskoedijk rhyskoedijk marked this pull request as ready for review October 15, 2024 08:35
@rhyskoedijk rhyskoedijk requested a review from a team as a code owner October 15, 2024 08:35
@brettfo
Copy link
Contributor

brettfo commented Oct 17, 2024

@rhyskoedijk A change was just merged that edited two of the files that were renamed, causing a conflict. Could you resolve the conflicts and update this PR? I'll hold off other C# changes until this is in so we don't keep stepping on your toes.

@rhyskoedijk
Copy link
Contributor Author

@brettfo I've updated the PR; one of the smoke tests failed due to a network error, is it possible to re-queue it?

@randhircs randhircs force-pushed the feature/tidy-packageconfig-packagereference-separation branch from 9a35a2b to 19477a5 Compare October 18, 2024 14:02
@randhircs randhircs force-pushed the feature/tidy-packageconfig-packagereference-separation branch from 19477a5 to 2e2c8bd Compare October 18, 2024 14:33
@randhircs randhircs merged commit 1c4db24 into dependabot:main Oct 18, 2024
51 checks passed
@rhyskoedijk rhyskoedijk deleted the feature/tidy-packageconfig-packagereference-separation branch October 19, 2024 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants