knative / test-infra

Test infrastructure for the Knative project
Apache License 2.0
82 stars 163 forks source link

Refactor selector with feedback from 3696 #3704

Closed evankanderson closed 1 year ago

evankanderson commented 1 year ago

I decided to follow up on my comments on #3696 for filtering Buoy modules.

I figured writing the refactor was the easiest way to communicate my ideas; this removes the need for MatcherFunc and simplifies the calling inferface for Module and Modules in a few places. I'm considering replacing Selector with 'In()andExcept, but I'm not sure it is a substantial improvement. (Excludesis only used in one place, andIn(string...)andExcept(Matcher, Matcher)` would cover the same cases.)

/assign @cardil

krsna-m commented 1 year ago

@cardil looks good to you?

evankanderson commented 1 year ago

I think Chris is only on mobile this week, according to another message on Slack.

This is anti-urgent, so I think it can wait.

I simplified the code one more time, removing Selector as an exported type in favor of returning an anonymous function which captures the "module under prefix but not in my current workspace" requirement.

knative-prow[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, evankanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/knative/test-infra/blob/main/OWNERS)~~ [cardil,evankanderson] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
knative-prow[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cardil, evankanderson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/knative/test-infra/blob/main/OWNERS)~~ [cardil,evankanderson] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment