-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Use ServiceLoader for ObjectFactory #1463
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
Use ServiceLoader for ObjectFactory #1463
Conversation
|
||
public class ObjectFactoryLoader { | ||
|
||
private static final ServiceLoader<ObjectFactory> LOADER = ServiceLoader.load(ObjectFactory.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The service loader is not safe for use by multiple concurrent threads. Runners are executed in parallel and each runner will create it's own backends with object factory. You can either make all fields and methods non-static or move this field into the method.
* | ||
* @deprecated as of version 4.0.0; use {@code loadObjectFactory() } instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While currently not package private, the ObjectFactoryLoader will be after merging. There is no need to keep a deprecated method around.
Looks exactly as expected! Thanks! Btw. You'll notice that by adding whitespace changes in files that you otherwise didn't touch you'll now have merge conflicts. While I really appreciate the sentiment of cleaning things up, try to make these changes only in combination with other changes. |
yep i auto format style often without thing, sometimes i only do it so it appears in git as a file i need to go back to check later. undone changes |
…ervice-locator # Conflicts: # core/src/test/java/cucumber/runtime/model/CucumberFeatureTest.java
You may want to revert that commit and instead use upstream/develop-v5 |
I cherry picked your changes into a03dd01. Cheers! |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Is this the type of changes you where expecting for #1450?
Details
Build fails at spring tests, but before I send more time is this what you where thinking?
How Has This Been Tested?
Not fully tested yet, tests are failing. Still work in process...
Types of changes
Checklist: