Skip to content
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

Extraction of Globals.prefs.put and .get #7121

Merged
merged 31 commits into from
Dec 14, 2020
Merged

Extraction of Globals.prefs.put and .get #7121

merged 31 commits into from
Dec 14, 2020

Conversation

calixtus
Copy link
Member

This PR aims to extract the calls to Globals.prefs.put and .get out of the JabRef codebase in preparation for some architectural healing...

  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

calixtus and others added 12 commits November 21, 2020 17:24
* upstream/master: (36 commits)
  Fix remembering password for sql db (#7154)
  Update to libre office 7.0.3 (#7150)
  Add IdBasedSearchFetcher to jstor (#7145)
  Squashed 'src/main/resources/csl-styles/' changes from 55200d0..a20406d
  Bump antlr4-runtime from 4.8-1 to 4.9 (#7136)
  Bump antlr4 from 4.8-1 to 4.9 (#7138)
  Bump mariadb-java-client from 2.7.0 to 2.7.1 (#7134)
  Bump classgraph from 4.8.90 to 4.8.92 (#7139)
  Bump mockito-core from 3.6.0 to 3.6.28 (#7135)
  Bump gittools/actions from v0.9.6 to v0.9.7 (#7144)
  Bump checkstyle from 8.37 to 8.38 (#7142)
  Add missing author
  Fix document viewer not showing first page (#7132)
  Add githandler mock to crawler test to fix NPE (#7133)
  Searchbar glyph icon colors in Dark Theme [FIXED] (#7131)
  Fix binding issue for the regex and case sensitive search buttons (#7125)
  Enable automated cross library search using a cross library query lan… (#7124)
  Add tracking
  Update Java Version
  Welcome Dominik ✌
  ...
@Siedlerchr
Copy link
Member

I added some further extractions and removed some calls to JabRefPreferences.getInstance(). TODO: We need some kind of MrDlib Preferences for SEND_LANGUAGE_DATA etc.

@Siedlerchr
Copy link
Member

Siedlerchr commented Dec 7, 2020

We should wait with merging until #6187 to prevent merge conflicts

@calixtus
Copy link
Member Author

calixtus commented Dec 9, 2020

This is definitely enough...
Poor @tobiasdiez having to review everything. 😉
But seriously, this is mostly refactoring, rewording, moving around etc.

Next step will be to extract all the calls to Globals.prefs.
I'm still thinking on how to deal with the preference migrations.

@calixtus
Copy link
Member Author

calixtus commented Dec 9, 2020

The localization consistency tests are parsing comments too...

@calixtus calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 9, 2020
@Siedlerchr
Copy link
Member

Can you test #7147 as well? Seems like also it's related to the prefs somehow

@calixtus calixtus marked this pull request as ready for review December 10, 2020 07:38
ClipBoardManager.clipboard = clipboard;
ClipBoardManager.primary = primary;
ClipBoardManager.importFormatReader = importFormatReader;

Copy link
Member

Choose a reason for hiding this comment

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

I guees Tobi or Olli will complain about the empty space ;)

Suggested change

Copy link
Member

Choose a reason for hiding this comment

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

No here it's fine...just don't add empty lines at the beginning or end of a method

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

This looks very nice! Thanks for all the hard work you put into the preferences. Let's merge to reduce the chance that incoming PRs create merge conflicts.

@calixtus
Copy link
Member Author

Something's off with the tests. I'll look into it

@koppor koppor merged commit c0153bd into master Dec 14, 2020
@koppor koppor deleted the refactor-prefs-calls branch December 14, 2020 20:28
Siedlerchr added a commit that referenced this pull request Dec 21, 2020
* upstream/master: (33 commits)
  Bump archunit-junit5-api from 0.14.1 to 0.15.0 (#7220)
  Bump unoloader from 7.0.3 to 7.0.4 (#7214)
  Bump guava from 30.0-jre to 30.1-jre (#7218)
  Bump xmpbox from 2.0.21 to 2.0.22 (#7217)
  Bump classgraph from 4.8.94 to 4.8.97 (#7211)
  Bump byte-buddy-parent from 1.10.18 to 1.10.19 (#7216)
  Bump archunit-junit5-engine from 0.14.1 to 0.15.0 (#7215)
  Bump org.beryx.jlink from 2.22.3 to 2.23.0 (#7212)
  Add missing author
  Remove field check for journal abbrev in entry editor (#7208)
  Improvements for Entry Preview (in the context of #7083 and in addition to #7093) (#7185)
  Fix pdf content importer exception if DOI is empty (#7207)
  New translations JabRef_en.properties (Turkish) (#7204)
  New Crowdin updates (#7198)
  New Crowdin updates (#7192)
  Added missing test
  Changed tests to parameterized tests
  Extraction of Globals.prefs.put and .get (#7121)
  Fix newly added entry not synced to db (#7178)
  Bump org.eclipse.jgit from 5.9.0.202009080501-r to 5.10.0.202012080955-r (#7187)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants