Skip to content

Conversation

@calixtus
Copy link
Member

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@calixtus calixtus marked this pull request as draft May 23, 2024 04:48
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@JabRef JabRef deleted a comment from github-actions bot May 23, 2024
@calixtus calixtus changed the title DI DI-1 May 23, 2024
@koppor
Copy link
Member

koppor commented May 27, 2024

This is an alternative to use Google's Dagger , which was tried out 2.5 years ago at #518

@koppor koppor mentioned this pull request May 27, 2024
5 tasks
@koppor
Copy link
Member

koppor commented May 27, 2024

We can use this PR to do a proper renaming:

Please fix the name "PreferenceService".

Reason:

@koppor
Copy link
Member

koppor commented May 27, 2024

I think, this is the right place to provide the link to some naming hints and usages JabRef#6103 (comment)

# Conflicts:
#	src/main/java/org/jabref/gui/fieldeditors/OptionEditor.java
#	src/main/java/org/jabref/gui/util/component/TemporalAccessorPicker.java
@calixtus
Copy link
Member Author

calixtus commented May 27, 2024

We can use this PR to do a proper renaming:

Please swap the names "PreferenceService" and "Preferences".

Can we postpone this to a follow up PR? I believe this one is on the edge of what can be understood in one PR.
Thats why I called the PR DI-1 😄
I just wanted to finally get rid of Globals and prepare the next steps by unifying the access and the injection method.

isDebugEnabled = false;
}

// addLogToDisk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment no action needed:

Here, we see different coding styles. I used method names to avoid comments and to enable grouping of related statements. For you, it seemed that grouping is too fine grained and you wanted to group using lines. -- For me having the single line comment // addLogToDisk spanning statements separated by empty lines was unclear (also because there is the comment The "Shared File Writer" ....

@calixtus calixtus closed this May 27, 2024
@koppor
Copy link
Member

koppor commented May 27, 2024

Follow-up at JabRef#11342

@koppor koppor deleted the di branch May 27, 2024 19:36
@calixtus calixtus mentioned this pull request May 27, 2024
6 tasks
@koppor
Copy link
Member

koppor commented May 27, 2024

We can use this PR to do a proper renaming:
Please swap the names "PreferenceService" and "Preferences".

Can we postpone this to a follow up PR?

Please directly after merge as DI-2.

@koppor koppor mentioned this pull request May 29, 2024
6 tasks
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