search

zendframework / zend-code

BSD 3-Clause "New" or "Revised" License
1.68k stars 78 forks source link

Corrected the exception class used and removed unnecessary strtolower in MethodScanner::setVisibility #140

Closed fhein closed 6 years ago

fhein commented 7 years ago

Exception - which is used as exception class within setVisibility - is not a class but a (sub)namespace. InvalidArgumentException likely is the intended exception class here.

This is my first submission. Wanted to get my feet wet with the submission workflow. :)

Ocramius commented 7 years ago

@fhein patch looks fine, but a test case verifying the expected thrown exception type is needed. See https://github.com/zendframework/zend-code/blob/6b1059db5b368db769e4392c6cb6cc139e56640d/test/Scanner/MethodScannerTest.php

You can push to your current branch to update this PR :+1:

fhein commented 6 years ago

Done. Plus a bit. :) Anything left to do?

Ocramius commented 6 years ago

Looking good!

fhein commented 6 years ago

:) . This was very helpful to setup and test my environment. Thanks.

After a short review of Roave/BetterReflection I think it is better to stop fiddling around with the scanner stuff here. I've got some spare time I could spent helping out. If you like and if there's demand, feel free to give me a try and direct me to somewhere where help is needed.

weierophinney commented 6 years ago

Thanks, @fhein!