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

Make Checker extendable from outside of the package #65

Closed
wants to merge 1 commit into from

Conversation

pickypg
Copy link

@pickypg pickypg commented Sep 4, 2015

Currently, using this.getClass() forces the abstract class to get the extension's package to look for resources. This change forces it to use Checker.class so that extension code can work.

I was tempted to make the parseBundledSignatures function non-final so that extensions could provide their own bundled resources as well, but I do not need that functionality yet.

@uschindler
Copy link
Member

Hi @pickypg, I'll look into this! This explains to my why you forked the Checker class.

@uschindler uschindler added this to the 1.9 milestone Sep 4, 2015
@uschindler uschindler self-assigned this Sep 4, 2015
@uschindler uschindler removed this from the 1.9 milestone Sep 4, 2015
@uschindler
Copy link
Member

Based on the discussion in #66, I will close this pull request as won't fix. It was added because of a "bug" in Checker: It used this.getClass() instead of a hardcoded Checker.class to load the bundled signatures. If somebody subclasses in a different package, the signatures won't load anymore.
#67 made the Checker class final, so this is no longer an issue, but I will change the getClass() anyways in a separate commit. Thanks for reporting!

@uschindler uschindler closed this Sep 4, 2015
uschindler added a commit that referenced this pull request Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants