Tighten behavior #5
Conversation
to handle dir-based spec, must consider path then dirname; flysystem's relative path behavior should mean a spec of '.' and './' will always match a candidate; highlight how a spec *can* match a dir, unlike Finder's behavior of never finding a dir;
tests/unit/FinderTest.php
Outdated
| ] | ||
| "dirname" => ".hiddendir", | ||
| "basename" => ".test.txt", | ||
| "filename" => ".text", |
There was a problem hiding this comment.
Shouldn't the filename be .test?
There was a problem hiding this comment.
I have no comments on this. Looks good.
Only wondering how it will behave on windows. Since the forward backward slash difference.
I think flysystems covers you. But our users could expect something different.
I think it makes sense to have some integration test with a real directory mounted in flysystem to check the behavior.
|
It could be argued that I went overboard on the |
| * Initializes the fixture for this test. | ||
| * @covers ::__construct | ||
| * @covers ::isSatisfiedBy | ||
| * @covers ::<private> |
There was a problem hiding this comment.
I recommend putting an @covers ::<private> in the class docblock since it makes little sense to add this to each test; nor to specify with each test which private method was covered
| ])); | ||
| } | ||
|
|
||
| protected function useWildcardPath() |
There was a problem hiding this comment.
Unless this function is used in a subclass I recommend making the visibility private. The project's recommendations are to use final classes and private methods unless there is a clear use-case to make it more visible. This will ensure a stable API and allow us to be flexible in which methods we change without breaking BC
|
Looks really good, I love the additions. Regarding the integration test; better to be thorough then to test too little; that looks awesome to me. One future boyscout activity could be to add more typehints to the parameters and return of the signature. But this does not impact this PR AFAIC |
|
I'll look at the typehinting next, after this merges. |
adjustments to behavior based on library expectations, flysystem's relative filesystem behavior, and issues seen when debugging
-dand-iin phpdocumentor