Closed trautmane closed 9 months ago
Since this was bugging me way more than it should (even during a workout), I went ahead and tried to make StackIdNamingGroup
never return null
. The choice of making it return a Pattern
rather than a String
is debatable, but I think the logic is much clearer now. In particular, I think this eliminated a flaw in the logic before the changes:
return ((cpp == null) || cpp.matcher(stackId.getProject()).matches()) && // project matches and
(((csp != null) && csp.matcher(stackId.getStack()).matches()) || // ( stack matches or
((stackNames != null) && stackNames.contains(stackId.getStack()))); // stack is in list )
If both csp
and stackNames
are null
, this expression evaluates to false
, even though csp == null
should mean that everything matches, if I understand this correctly.
What do you think?
Thanks for fixing the string isEmpty calls to isBlank!
I also thought the use of Predicates was interesing - I did not realize Pattern.asMatchPredicate()
existed. However, the Predicates and the MATCH_ALL constant require Pattern and Matcher objects to be created even when there is no pattern string. I intended/hoped to avoid that cost, though I admit it likely is an insignificant cost in most cases.
I also don't find the new solution more readable. Yes, the filter(groupMembershipTest)
call is nicer, but I think the complexity is just pushed into the getMembershipTest()
method and I find that implementation harder to reason about than the simple null checks.
I'll stop by a little later to discuss your thoughts on this feedback and we can settle on a final solution.
for multi-SEM pipelines so that multiple stacks (within multiple projects) can be processed concurrently.
This is very much a work-in-progress, but I thought it would still be useful to push/review early drafts.