Use regular expressions in the blacklist for more convenient matching. #13
Conversation
|
Currently lacks tests, but i'm happy to add those if you agree with this approach. |
|
The idea looks good to me. Next time you rebase, you will see some merge conflicts. I could see these tests cases in addition to many others you can add.
The PR should also include documentation. Please extend the Readme accordingly. Thanks for the hard work. |
|
Thanks for the feedback. I'll cleanup the PR, add support for regex'ing the whitelist and add test cases. |
pieterlange
force-pushed
the
feature/regexp-blacklist
branch
from
4191333 to
e3d0cca
Compare
|
Updated. This can probably be refactored but i'm not a big golang whiz. 😬 Tests will follow soon. |
|
I don't have the time to figure out how to properly write tests for this feature. Would appreciate some help here. 😬 Sorry. But i also don't want to keep people waiting for this feature. |
|
HI @pieterlange, Golang has enormous amount of resources about unit test frameworks. This can be a starting point. https://golang.org/pkg/testing/ We would appreciate test coverage of the new added feature. I mentioned a starter list of tests here #13 (comment) |
| } | ||
|
|
||
| // filter iterates through the list of all files in the repo and filters it | ||
| // down to a list of those that should be applied. | ||
| func filter(rawApplyList, blacklist, whitelist []string) []string { | ||
| blacklistMap := stringSliceToMap(blacklist) | ||
| whitelistMap := stringSliceToMap(whitelist) | ||
| //whitelistMap := stringSliceToMap(whitelist) |
There was a problem hiding this comment.
Please do not leave commented out code in the commit.
| @@ -28,28 +28,28 @@ func TestPurgeComments(t *testing.T) { | |||
| }{ | |||
| // No comment | |||
| { | |||
| []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, | |||
| []string{"/repo/a/b.json", "/repo/b/c", "/repo/a/b/c.yaml", "/repo/a/b/c", "/repo/c.json"}, | |||
| []string{"a/b.json", "b/c", "a/b/c.yaml", "a/b/c", "c.json"}, | |||
There was a problem hiding this comment.
Why did you change the repo prefix ?
What was the problem with it ?
|
Is there any chance this will ever get merged? |
|
I moved away from So i dont use this anymore. |
Start using a new kube-applier that can be executed by a non-root user
|
|
Resolves #10
Comments welcome.