Replace new instantiation of BuildInfo with calls to Injector#11954
Replace new instantiation of BuildInfo with calls to Injector#11954
Conversation
koppor
left a comment
There was a problem hiding this comment.
Looks good - you decided against constructor-based injection, but sued the Injector directly? If it is OK for you, it is OK for me!
|
I have no hard opinion on this atm. Probably think about some hours. Will come back to this later |
|
I think, constructor-based injection will increase the code, but the dependencies are explicit. In constract, with the injector, one "just" needs to search for class usages with an Injector... Using the example of |
|
In general, constructor based dependency injection is preferable because you can better test it and it's more explicit. For example, spring also recommends ctor based injection |
|
The build of this PR is available at https://builds.jabref.org/pull/11954/merge. |
koppor
left a comment
There was a problem hiding this comment.
I like keeping the preferences together. - Some code still needs to be adapted to use
importerPreferences.getApiKey(getName())
instead of
Injector.instantiateModelOrService(BuildInfo.class).semanticScholarApiKey
because the keys are now provided by importerPreferences.getApiKey
|
Then what is the purpose of DI framework? Like the one we use? |
|
Afterburner is not a di framework. |
|
The build of this PR is available at https://builds.jabref.org/pull/11954/merge. |
|
The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build. |
closes https://github.com/JabRef/jabref-issue-melting-pot/issues/604
also removes some telemetry artifacts
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)