-
Notifications
You must be signed in to change notification settings - Fork 706
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
Added Embedded Resource Capability #6303
Added Embedded Resource Capability #6303
Conversation
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/Package/EmbeddedResourcesCapability.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/Package/EmbeddedResourcesCapability.cs
Outdated
Show resolved
Hide resolved
private INuGetPackageFileService _nugetPackageFileService; | ||
private PackageModel _package; | ||
|
||
public EmbeddedResourcesCapability(INuGetPackageFileService nugetPackageFileService, PackageModel package, Uri? iconUri, Uri? licenseUri, Uri? readmeUri) |
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 it's possible to have a License and not a README or icon, should these be separate classes instead? Otherwise, should these be required parameters?
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.
It's possible, but the capabilities are added in the package definitions, right? I don't know if there's a type of package that only includes one of the embedded resources.
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/Package/EmbeddedResourcesCapability.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/Package/EmbeddedResourcesCapability.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
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.
Approved with suggestions.
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
...nts.Tests/NuGet.PackageManagement.UI.Test/Models/Package/EmbeddedResourcesCapabilityTests.cs
Outdated
Show resolved
Hide resolved
src/NuGet.Clients/NuGet.PackageManagement.UI/Models/Package/EmbeddedResourcesCapability.cs
Outdated
Show resolved
Hide resolved
edc3394
into
dev-feature-PackageDomainModel
Bug
Fixes: https://github.com/NuGet/Client.Engineering/issues/3198
Description
Added new capability for downloading embedded resources for packages.
PR Checklist
Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.