Guard-Assertions #172
Guard-Assertions #172
Conversation
|
@UFOMelkor the patch looks really good! There's just one thing that bothers me, and it is that we're duplicating code between the Controller and the Route guard - can you try to refactor that out? |
|
@Ocramius An AbstractGuard-class would be ok for you? |
|
@UFOMelkor I think that could do the trick, yes |
|
Please check if you can also reduce the amount of test code starting from that |
|
I will continue at home, got some trouble with running tests here |
|
@Ocramius |
|
@UFOMelkor won't nitpick further on the test cases. But a rebase will be needed :-) |
|
Changes Unknown when pulling 53b1eb2 on UFOMelkor:feature/assertions-for-guards into * on bjyoungblood:master*. |
|
@Ocramius Does it look good so? It was my first rebase … |
| $this->serviceLocator = $serviceLocator; | ||
|
|
||
| foreach ($rules as $rule) { | ||
| $rule['roles'] = (array) $rule['roles']; |
There was a problem hiding this comment.
Align = signs, remove empty newline after this line
|
Is the abstract class fully covered? I saw coverage decreased |
|
Changes Unknown when pulling c459f9b on UFOMelkor:feature/assertions-for-guards into * on bjyoungblood:master*. |
|
@Ocramius I handled your suggestions. The AbstractGuard is fully covered, coverage decreased because of less lines. |
|
I'll look at the cs fix. |
|
Changes Unknown when pulling de48c02 on UFOMelkor:feature/assertions-for-guards into * on bjyoungblood:master*. |
|
@Ocramius fixed :-) |
|
Thanks! |
|
Same to you. Makes really fun to contribute to repositories supervised by you 👍 |
|
@UFOMelkor I thought it was hell, given my aggressive reviews :-) |
|
@Ocramius Reviews are just technical, not personal und they are a great chance to improve my skills :-) |
Makes it possible to specify assertions direct at guard configuration.