-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Observable Preferences V (SavePreferences) #9808
Conversation
# Conflicts: # CHANGELOG.md
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java
Outdated
Show resolved
Hide resolved
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.
Codewise, it looks good. Nitpick at the Changelog :p
However, we could have a performance issue on saving. At first time, I tried this, saving on my network drive took ages. Checking out main
and then saving was fast. Going back to the PR: Saving was fast again. Thus, maybe not an issue of this PR, but I just wanted to note it down.
Co-authored-by: Oliver Kopp <[email protected]>
About speed: please try gradle clean and then run again. |
Tried it and no performance issues. Might have been a hackup at first try. |
Follow up to #9619
Probably my worst one yet: 83 files changed. As the whole complex was tangled into itself parting it in multiple PRs was not reasonable. Again easiest to review PR by PR.
Moved one setting from file tab to general tab, as it is a behavioral one.
Changed the way of SavePreferences to SaveConfiguration, since i believe it was just a DTO.
Compulsory checks