Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create reflection parameter from closure #256

Merged
merged 2 commits into from
Mar 31, 2017

Conversation

marcosh
Copy link

@marcosh marcosh commented Mar 31, 2017

solves #255

Copy link
Member

@asgrim asgrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a useful change, just couple of minor fixes please! 👍

* @param string $parameterName
* @return ReflectionParameter
*/
public static function createFormClosure(\Closure $closure, $parameterName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • typo in method name createFromClosure
  • also please add type declarations (string $parameterName and : ReflectionParameter) (I'm working on adding type declarations everywhere else as we're min PHP 7 for next release now)

{
$parameterInfo = ReflectionParameter::createFormClosure(function ($a) {}, 'a');

$this->assertInstanceOf(ReflectionParameter::class, $parameterInfo);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use self::assertInstanceOf

$parameterInfo = ReflectionParameter::createFormClosure(function ($a) {}, 'a');

$this->assertInstanceOf(ReflectionParameter::class, $parameterInfo);
$this->assertSame('a', $parameterInfo->getName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use self::assertSame

@asgrim asgrim added this to the 1.3.0 milestone Mar 31, 2017
@asgrim asgrim self-assigned this Mar 31, 2017
@marcosh
Copy link
Author

marcosh commented Mar 31, 2017

fixed the minor changes

Copy link
Member

@asgrim asgrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice - thanks for the patch 😄

@asgrim asgrim merged commit 990c879 into Roave:master Mar 31, 2017
@asgrim asgrim modified the milestones: 1.3.0, 2.0.0 Mar 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants