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

support multi-part requirements #10255

Merged
merged 2 commits into from
Jul 22, 2024
Merged

support multi-part requirements #10255

merged 2 commits into from
Jul 22, 2024

Conversation

brettfo
Copy link
Contributor

@brettfo brettfo commented Jul 19, 2024

Requirements passed to an update operation (e.g., as a part of the ignored_versions property) can have multiple parts separated by a comma. These are interpreted as the AND combination of each individual requirement. The NuGet updater doesn't support this, so this PR adds that functionality.

The Requirement class was made abstract with two concrete classes inheriting from it; IndividualRequirement and MultiPartRequirement.

IndividualRequirement is exactly the same as the original Requirement class, modulo moving some common code into the base class.

MultiPartRequirement simply contains multiple IndividualRequirements and when checking for version compatibility, returns the AND combination of all parts via the LINQ method All().

@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Jul 19, 2024
@brettfo brettfo marked this pull request as ready for review July 19, 2024 22:02
@brettfo brettfo requested a review from a team as a code owner July 19, 2024 22:02
@brettfo brettfo marked this pull request as draft July 20, 2024 00:18
@brettfo brettfo marked this pull request as ready for review July 20, 2024 00:24
jeffwidman
jeffwidman approved these changes Jul 22, 2024
View reviewed changes
Copy link
Member

@jeffwidman jeffwidman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually this looks reasonable to me.

However, I don't know nuget/C#, so could you please have one of your teammates review before we deploy/merge?

@abdulapopoola abdulapopoola merged commit 5ef19be into main Jul 22, 2024
67 checks passed
@abdulapopoola abdulapopoola deleted the dev/brettfo/nuget-multi-part-requirements branch July 22, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffwidman jeffwidman jeffwidman approved these changes

@JamieMagee JamieMagee JamieMagee approved these changes

Assignees
No one assigned
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@brettfo @jeffwidman @JamieMagee @abdulapopoola