Add a new Match method to the reference package#2039
Add a new Match method to the reference package#2039stevvooe merged 1 commit intodistribution:masterfrom vdemeester:add-match-support-to-reference
Conversation
Current coverage is 51.38% (diff: 100%)@@ master #2039 diff @@
==========================================
Files 126 126
Lines 11275 11268 -7
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 6939 5790 -1149
- Misses 3449 4732 +1283
+ Partials 887 746 -141
|
| reference: "foo/c/baz:tag", | ||
| pattern: "foo/c/baz:tag", | ||
| expected: true, | ||
| }, |
There was a problem hiding this comment.
Let's add a few test cases for references that include domain names.
| return false | ||
| } | ||
| if namedRef, isNamed := ref.(Named); isNamed && !matched { | ||
| matched, _ = path.Match(pattern, namedRef.Name()) |
There was a problem hiding this comment.
I'm confused about why Match is called twice. Seems like this is returning matches(ref.String()) || matches(ref.Name()). Why isn't ref.String() enough?
Also, the eror is not checked here.
There was a problem hiding this comment.
This is returning matches(ref.String()) || matches(ref.Name()) if the reference is Named, others ref will only check ref.String(). The basic idea is to map a little bit what docker images did (and a little more). That means foo/*/ba[rz] would match foo/a/bar:tag for example. To achieve that, I need to check if the name part matches if the whole string does not (this way foo/*/ba[rz]:t* will match foo/a/bar:tag too).
The error is not checked here because it cannot happen the only error is if the pattern is wrong, and this error is already checked just before 👼.
There was a problem hiding this comment.
It just seems confusing to me that a function called Match would have this behavior. Could we call it something like MatchRefOrName? Feel free to suggest something better.
There was a problem hiding this comment.
@aaronlehmann ah I see what you mean, makes sence. I'll rename 👼
| }, nil | ||
| } | ||
|
|
||
| // Match reports whether ref matches the specified pattern. |
There was a problem hiding this comment.
Can you add link to match pattern syntax https://golang.org/pkg/path/#Match
reference/reference.go
Outdated
|
|
||
| // Match reports whether ref matches the specified pattern. | ||
| // See https://godoc.org/path#Match for supported patterns. | ||
| func Match(pattern string, ref Reference) bool { |
There was a problem hiding this comment.
Let's follow the same pattern as the stblib: this should return an error if the pattern is bad.
|
LGTM |
reference/reference.go
Outdated
|
|
||
| // MatchRefOrName reports whether ref matches the specified pattern. | ||
| // See https://godoc.org/path#Match for supported patterns. | ||
| func MatchRefOrName(pattern string, ref Reference) (bool, error) { |
There was a problem hiding this comment.
Let's just call it match, please.
The Match method allows to see if a reference matches a specified patterns. Signed-off-by: Vincent Demeester <[email protected]>
|
@stevvooe updated 👼 |
|
LGTM |
7 participants
The Match method allows to see if a reference matches a specified patterns.
This is linked to moby/moby#27872 (comment) 👼
/cc @stevvooe @aaronlehmann
🐸
Signed-off-by: Vincent Demeester [email protected]