Skip to content

Observable Preferences V (SavePreferences)#9808

Merged
Siedlerchr merged 26 commits into
mainfrom
SavePreferences
Apr 30, 2023
Merged

Observable Preferences V (SavePreferences)#9808
Siedlerchr merged 26 commits into
mainfrom
SavePreferences

Conversation

@calixtus

@calixtus calixtus commented Apr 27, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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.
grafik

Changed the way of SavePreferences to SaveConfiguration, since i believe it was just a DTO.

### Compulsory checks
- [x] Change in `CHANGELOG.md` described in a way that is understandable for the average user (if applicable)
- [ ] Tests created for changes (if applicable)
- [x] Manually tested changed features in running JabRef (always required)
- [x] Screenshots added in PR description (for UI changes)
- [ ] [Checked developer's documentation](https://devdocs.jabref.org/): Is the information available and up to date? If not, I outlined it in this pull request.
- [ ] [Checked documentation](https://docs.jabref.org/): 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 added 20 commits April 22, 2023 15:00
@calixtus
Reworded SafeOrderConfig
5d0bb90
@calixtus
Extracted CitationKeyWriterPreferences from SavePreferences
427cbf2
@calixtus
Extracted CitationKeyWriterPreferences from SavePreferences
e1d4c4a
@calixtus
Extracted FileWriterPreferences from SavePreferences
bdc9d45
@calixtus
Removed FieldContentFormatterPreferences and reworded FieldPreferences
c6b427a
@calixtus
Reworded
3d11440
@calixtus
Removed saveInOriginalOrder and fixed one test
9f61a2a
@calixtus
Fixed tests
f74a151
@calixtus
Reworded and prevented npe
f9ed5a1
@calixtus
Reworded and moved DatabaseSaveType
51ce8c8
@calixtus
Reworded SavePreferences to SaveConfiguration
1b91175
@calixtus
Fixed a test
7cd69bf
@calixtus
Extracted getSavePreferences
a7622d2
@calixtus
Reworded SavePreferencesForExport
59a52d0
@calixtus
Extracted FieldPreferences
82af5d1
@calixtus
Moved shouldOpenLastEdited to GeneralPreferences
6265afe
@calixtus
CHANGELOG.md
e6d4b0f
@calixtus
Refactored ExportSaveOrder to new prefs pattern
349d463
@calixtus
Merge remote-tracking branch 'upstream/main' into SavePreferences
bff47d2 
# Conflicts:
#	CHANGELOG.md
@calixtus
CHANGELOG.md
bd4f844
github-actions[bot]
github-actions Bot reviewed Apr 27, 2023
View reviewed changes
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
Comment thread src/test/java/org/jabref/logic/importer/fetcher/ArXivFetcherTest.java Outdated
@Siedlerchr Siedlerchr changed the title Observable Preferences V (SafePreferences) Observable Preferences V (SavePreferences) Apr 27, 2023
@calixtus calixtus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers dev: code-quality Issues related to code or architecture decisions component: preferences labels Apr 27, 2023
koppor
koppor requested changes Apr 28, 2023
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

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.

Comment thread CHANGELOG.md Outdated
calixtus and others added 2 commits April 29, 2023 16:30
@calixtus @koppor
Update CHANGELOG.md
ab8fdbf 
Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
@calixtus
Fixed preposition
3ae7988
@calixtus

calixtus commented Apr 29, 2023

Copy link
Copy Markdown
Member Author

About speed: please try gradle clean and then run again.

@koppor

koppor commented Apr 30, 2023

Copy link
Copy Markdown
Member

About speed: please try gradle clean and then run again.

Tried it and no performance issues. Might have been a hackup at first try.

@Siedlerchr Siedlerchr merged commit 913b46c into main Apr 30, 2023
@Siedlerchr Siedlerchr deleted the SavePreferences branch April 30, 2023 13:55
@calixtus calixtus mentioned this pull request Jun 11, 2023
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@koppor koppor koppor approved these changes

Assignees

No one assigned

Labels

component: preferences dev: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@calixtus @koppor @Siedlerchr