Skip to content

Alter the PointcutInterface::matchesMethod to include the ClassReflectio...#13

Open
rande wants to merge 1 commit intoschmittjoh:masterfrom
ekino:refactor_pointcut_interface
Open

Alter the PointcutInterface::matchesMethod to include the ClassReflectio...#13
rande wants to merge 1 commit intoschmittjoh:masterfrom
ekino:refactor_pointcut_interface

Conversation

@rande
Copy link
Copy Markdown

@rande rande commented Aug 20, 2012

...n instance

The current PointcutInterface methods signature is only valid if one Pointcut
handle one class. However if a Pointcut is implemented to handle more than
one class then the model is broken. The MethodReflection contains the class
where the method is defined and not the class referenced in a sub type class.

So this patch solves the issue by providing the ReflectionClass targetted by
the configuration.

…tion instance

The current PointcutInterface methods signature is only valid if one Pointcut
handle one class. However if a Pointcut is implemented to handle more than
one class then the model is broken. The MethodReflection contains the class
where the method is defined and not the class referenced in a sub type class.

So this patch solves the issue by providing the ReflectionClass targetted by
the configuration.
@travisbot
Copy link
Copy Markdown

This pull request fails (merged ad502a6 into 2b49b78).

@schmittjoh
Copy link
Copy Markdown
Owner

I'm wondering if we could make this BC by adding a second interface instead of changing the current interface?

@rande
Copy link
Copy Markdown
Author

rande commented Sep 6, 2012

The current method signature is broken. Adding a new interface will more confusing than solving the initial issue.

@schmittjoh
Copy link
Copy Markdown
Owner

I don't think that it is broken, but it has limitations if you depend on
config instead of annotations for metadata.

I'd suggest we deprecate it, and add another interface,
AdvancedPointcutInterface f.e.

If you try to follow semantic versioning, a BC break would mean a 2.0
release, and I'm not sure this bundle needs another major release just yet
:)

On Thu, Sep 6, 2012 at 5:58 PM, Thomas notifications@github.com wrote:

The current method signature is broken. Adding a new interface will more
confusing than solving the initial issue.


Reply to this email directly or view it on GitHubhttps://github.com//pull/13#issuecomment-8337213.

@rande
Copy link
Copy Markdown
Author

rande commented Sep 6, 2012

Well, It is a good day for release ;)

I will see how to do so, but this will not be before a few days.

@rande
Copy link
Copy Markdown
Author

rande commented Oct 2, 2012

@schmittjoh I can start to work on this PR again.

I will move the code to an AdvancedPointcutInterface, agreed ?

@schmittjoh
Copy link
Copy Markdown
Owner

Yes, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants