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

Some OO/LO cleanups #1927

Merged
merged 1 commit into from
Sep 11, 2016
Merged

Some OO/LO cleanups #1927

merged 1 commit into from
Sep 11, 2016

Conversation

oscargus
Copy link
Contributor

@oscargus oscargus commented Sep 6, 2016

Removed deprecated calls, renamed a few variables, and restructured the code a bit.

@oscargus oscargus added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers openoffice/libreoffice type: code-quality Issues related to code or architecture decisions labels Sep 6, 2016
@@ -1366,13 +1366,13 @@ public BibDatabase generateDatabase(List<BibDatabase> databases)
BibEntry clonedEntry = (BibEntry) entry.get().clone();
clonedEntry.setId(IdGenerator.next());
// Insert a copy of the entry
resultDatabase.insertEntryWithDuplicationCheck(clonedEntry);
resultDatabase.insertEntry(clonedEntry);
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering: Is it save to remove the duplication check here? The documentation of insertEntryWithDuplicationCheck says use insertEntry and DuplicationChecker separately.

The comment also applies to the replacement below.

@lenhard
Copy link
Member

lenhard commented Sep 6, 2016

Apart from the clarification above: LGTM!

@oscargus
Copy link
Contributor Author

oscargus commented Sep 6, 2016

Well the duplication check just returns a boolean and as that is not checked here it shouldn't make any functional difference. If it was a good idea in the first place is another question though...

@tobiasdiez
Copy link
Member

LGTM 👍

@tobiasdiez tobiasdiez merged commit fdfbcac into JabRef:master Sep 11, 2016
Siedlerchr added a commit that referenced this pull request Sep 11, 2016
* master:
  Remove obsolete wrapper task
  Added error dialog when setting invalid main file directory (#1921)
  Add filteringCharset = 'UTF-8' (#1945)
  Include https://github.com/grimes2
  Searchbar across all bib files instead each having its own (#1549)
  Some OO/LO cleanups (#1927)
  Update link
  Removed external dependency in logic (#1934)
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Sep 11, 2016
* master:
  Remove obsolete wrapper task
  Added error dialog when setting invalid main file directory (JabRef#1921)
  Add filteringCharset = 'UTF-8' (JabRef#1945)
  Include https://github.com/grimes2
  Searchbar across all bib files instead each having its own (JabRef#1549)
  Some OO/LO cleanups (JabRef#1927)
  Update link
  Removed external dependency in logic (JabRef#1934)
zesaro pushed a commit to zesaro/jabref that referenced this pull request Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openoffice/libreoffice status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: code-quality Issues related to code or architecture decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants