Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adds support for NuGet V2 upstream package sources #630
Adds support for NuGet V2 upstream package sources #630
Changes from all commits
8d7856f
ca2d5d0
101de99
8dff49d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Hey this is a tiny bit of feature creep, but I think you can leverage the options validation here by adding attributes on the
PackageSource
property: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.
Excellent idea. However when trying this it complains that only one
IfRequired
attribute can be used.As I workaround, we can do the following, that will work but will show the error message
Invalid 'Mirror' configs: 'PackageSource' is required because 'EnabledOrLegacy' has a value of 'True'.
which does not reflect the settings properly, but maybe this is OK?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.
Ah good catch! It looks like you can support multiple attribute usages if we:
AttributeUsageAttribute.AllowMultiple
to trueTypeId
property (see this)For an example of a validation attribute that supports multiple uses, see
CustomValidationAttribute
.I'll do this in a follow-up PR since this is minor and I've blocked your changes for a little too long already!
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.
Ideally this should be done at startup by ASP.NET Core's options validation. See my comment up above: https://github.com/loic-sharma/BaGet/pull/630/files#r582447197
If that idea works, you should be able to remove this check.