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

allow explicit priority config in SlicesRuleDefinition #1436

Merged

Conversation

guesshe
Copy link

@guesshe guesshe commented Mar 20, 2025

Resolves: #1434

@guesshe guesshe force-pushed the guesshe-explicit-priority-in-slicesruledefinition branch from 7f235cb to 84d08d6 Compare March 20, 2025 13:44
@hankem hankem force-pushed the guesshe-explicit-priority-in-slicesruledefinition branch from de37753 to 4583bb4 Compare April 13, 2025 10:49
Copy link
Member

@hankem hankem left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
I'll push a tiny fixup commit to simplify the previously existing code. If you agree, we can squash it into yours and merge the PR.

@@ -67,12 +67,28 @@ public GivenSlices matching(String packageIdentifier) {
return new GivenSlicesInternal(Priority.MEDIUM, Slices.matching(packageIdentifier));
Copy link
Member

Choose a reason for hiding this comment

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

We could reduce a tiny bit of code duplication if this now delegated to the new method:

Suggested change
return new GivenSlicesInternal(Priority.MEDIUM, Slices.matching(packageIdentifier));
return matching(packageIdentifier, Priority.MEDIUM);

public GivenSlices matching(String packageIdentifier, Priority priority) {
return new GivenSlicesInternal(priority, Slices.matching(packageIdentifier));
}

/**
* @see Slices#assignedFrom(SliceAssignment)
*/
@PublicAPI(usage = ACCESS)
public GivenSlices assignedFrom(SliceAssignment assignment) {
return new GivenSlicesInternal(Priority.MEDIUM, Slices.assignedFrom(assignment));
Copy link
Member

Choose a reason for hiding this comment

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

This could likewise delegate to the other new method:

Suggested change
return new GivenSlicesInternal(Priority.MEDIUM, Slices.assignedFrom(assignment));
return assignedFrom(assignment, Priority.MEDIUM);

@guesshe
Copy link
Author

guesshe commented Apr 14, 2025

Thanks for the contribution! I'll push a tiny fixup commit to simplify the previously existing code. If you agree, we can squash it into yours and merge the PR.

Sweet! Thanks for tidying it up!

@hankem hankem force-pushed the guesshe-explicit-priority-in-slicesruledefinition branch from 334bf78 to 17a1160 Compare April 14, 2025 18:40
@hankem hankem enabled auto-merge April 14, 2025 18:40
@hankem hankem merged commit 8282dc0 into TNG:main Apr 14, 2025
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is it possible to use fluent API to set priority for SlicesRuleDefinition? (Related to closed issue#129)
3 participants