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

Don't send extraneous 'handler' property on proxy #27

Merged
merged 1 commit into from
Oct 7, 2018

Conversation

nwholloway
Copy link
Contributor

Ignore properties of type javassist.util.proxy.MethodHandler present on the proxies generated by JavAssist.

Ignore properties of type javassist.util.proxy.MethodHandler present
on the proxies generated by JavAssist.
@nwholloway nwholloway requested a review from hdpe October 6, 2018 11:05
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 98.259% when pulling c2a9248 on nwholloway:fix/issue24 into 9e021c7 on BlackPepperSoftware:master.

@nwholloway
Copy link
Contributor Author

This should address issue #24

@hdpe hdpe merged commit c2a9248 into hdpe:master Oct 7, 2018
@hdpe
Copy link
Owner

hdpe commented Oct 7, 2018

What a tidy solution - thanks!

Code coverage was always a problem in this file. Making ResourceMixin subclass Resource was a "pattern" I nicked from some Spring code somewhere, just to give you a bit of a hand via compiler/IDE when overriding property serialisation. With no property serialisation overridden, I think the drawbacks of doing this now outweigh the benefits.

SO in the interests of consistency and removing non-private constructors, I took the liberty of adding a private constructor to your mixin, and replacing the ctor in the other mixin with an empty private one too. This should stop Coveralls complaining.

Thanks for the PR!

@nwholloway
Copy link
Contributor Author

Nice solution to the coverage.

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