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

WELD-2414 Fix InterceptedSubclassFactory #1725

Merged
merged 1 commit into from Sep 19, 2017

Conversation

@mkouba
Copy link
Member

commented Sep 18, 2017

  • also expand InterceptorBridgeMethodTest
@mkouba mkouba force-pushed the mkouba:WELD-2414 branch from 7cf3167 to 9497978 Sep 18, 2017
@mkouba mkouba changed the title WELD-2414 Attempt to fix InterceptedSubclassFactory WELD-2414 Fix InterceptedSubclassFactory Sep 18, 2017
@mkouba mkouba requested a review from manovotn Sep 18, 2017
@@ -437,4 +450,61 @@ private void createDelegateMethod(ClassFile proxyClassType, Method method, Metho
createDelegateToSuper(delegatingMethod, methodInformation);
}

}
private static class BridgeMethod {

This comment has been minimized.

Copy link
@manovotn

manovotn Sep 18, 2017

Contributor

I know it more or less doesn't matter, but wouldn't it be a tad bit nicer to have this as a separate class file which extends MethodSignature?

This comment has been minimized.

Copy link
@mkouba

mkouba Sep 18, 2017

Author Member

We can make it separate once it is usable outside InterceptedSubclassFactory. Also extending MethodSignature does not bring any value here. And by the way, it's not a "method signature" as it contains the return type ;-)

This comment has been minimized.

Copy link
@manovotn

manovotn Sep 18, 2017

Contributor

That's why it would be an 'extended' method signature, right? :-P

}

@Test
@Ignore("WELD-2424")

This comment has been minimized.

Copy link
@manovotn

manovotn Sep 18, 2017

Contributor

Just a side note - once we merge this, we should link this test to WELD-2424 issue so that it's clear how the use case looks like. Don't know about you, but I sure won't remember there was a test added back here :)

This comment has been minimized.

Copy link
@mkouba

mkouba Sep 18, 2017

Author Member

Yes, of course. I will add a link to this line once the PR is merged.

- also expand InterceptorBridgeMethodTest
@mkouba mkouba force-pushed the mkouba:WELD-2414 branch from 9497978 to 7f01542 Sep 19, 2017
@mkouba mkouba merged commit 20f6201 into weld:master Sep 19, 2017
2 checks passed
2 checks passed
Weld Central CI Finished-https://gist.github.com/3528143de5037c5564a87aed42d96146
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.