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

Configure compound classes to use JAR plugins first #294

Closed
dmitry-timofeev opened this issue Mar 4, 2019 · 4 comments
Closed

Configure compound classes to use JAR plugins first #294

dmitry-timofeev opened this issue Mar 4, 2019 · 4 comments
Milestone

Comments

@dmitry-timofeev
Copy link
Contributor

As documentation states that the JAR plugin format is recommended and shall be used by default, shan't the compound description finder, loader, and repository be configured with JAR versions first? It will (can't say how much) improve performance and reduce the number of log messages in the common case.

@decebals
Copy link
Member

decebals commented Mar 5, 2019

The first plugin format supported by PF4J was ZIP. In order to not affect the applications that already use this format (old PF4J version) for plugins, and to increase the adoption of the new PF4J versions, I made the decision to support this format out of the box (no modifications required).
If other users/developers vote to support only JAR plugins by default (and to specify implicitly when the application uses ZIP format for plugins) we can made the modifications.

@dmitry-timofeev
Copy link
Contributor Author

dmitry-timofeev commented Mar 6, 2019

I see, thanks! Such change is breaking for applications using both formats and relying on CompondX to pick the right format. But just to be clear: I do not propose to drop support of ZIP; rather to check for JAR in CompoundX managers first because JAR is the default format.

@decebals
Copy link
Member

decebals commented Mar 6, 2019

rather to check for JAR in CompoundX managers first because JAR is the default format.

Thanks for clarification. I think that is a good idea. A PR is welcome.

decebals added a commit that referenced this issue Mar 12, 2019
@decebals
Copy link
Member

I made the modifications on a new (feature) branch. I was forced to adapt a test to fix a fail. This test fail can be a problem for people, so I will investigate why the test failed:

java.lang.AssertionError
	at org.junit.Assert.fail(Assert.java:86)
	at org.junit.Assert.assertTrue(Assert.java:41)
	at org.junit.Assert.assertFalse(Assert.java:64)
	at org.junit.Assert.assertFalse(Assert.java:74)
	at org.pf4j.LoadPluginsTest.deletePlugin(LoadPluginsTest.java:243)

image

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

No branches or pull requests

2 participants