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.

Sign up for GitHub

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

Jump to bottom

Conflicting documentation on ISettings.AddOrUpdate #10980

Closed
krafs opened this issue Jun 30, 2021 · 2 comments · Fixed by NuGet/NuGet.Client#4151
Closed

Conflicting documentation on ISettings.AddOrUpdate #10980

krafs opened this issue Jun 30, 2021 · 2 comments · Fixed by NuGet/NuGet.Client#4151
Labels
Area:Settings NuGet.Config and related issues Category:Quality Week Issues that should be considered for quality week Community PRs (and linked Issues) created by someone not in the NuGet team Type:Bug Type:DeveloperDocs

Comments

@krafs
Copy link

krafs commented Jun 30, 2021

NuGet.Configuration.ISettings.AddOrUpdate returns void, but the method xml comment states that it returns true if the operation was successful.

Either the method should be changed to return a bool, or the documentation should be changed.
https://github.com/NuGet/NuGet.Client/blob/77e1bdb266093fd1eafff90ef21ad55a2a448889/src/NuGet.Core/NuGet.Configuration/Settings/ISettings.cs#L29-L30
@kartheekp-ms kartheekp-ms added Area:Settings NuGet.Config and related issues Type:Bug Category:Quality Week Issues that should be considered for quality week labels Jun 30, 2021
@zkat
Copy link
Contributor

zkat commented Jul 12, 2021

It's a public interface, so we can't really change it, which means this is really a docs issue. Would you mind sending us a PR to change that return value doc, since that's what's wrong? :) Feel free to ping me for a review.

@zkat zkat added the Community PRs (and linked Issues) created by someone not in the NuGet team label Jul 12, 2021
@krafs krafs mentioned this issue Jul 13, 2021
Merged
8 tasks
@krafs
Copy link
Author

krafs commented Jul 13, 2021

I figured as much, but didn't want to assume.
PR has been created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area:Settings NuGet.Config and related issues Category:Quality Week Issues that should be considered for quality week Community PRs (and linked Issues) created by someone not in the NuGet team Type:Bug Type:DeveloperDocs
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Removed return value from docs on ISettings.AddOrUpdate
4 participants
@zkat @heng-liu @krafs @kartheekp-ms