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

Revert deprecation #2023

Merged
merged 1 commit into from
Sep 22, 2016
Merged

Revert deprecation #2023

merged 1 commit into from
Sep 22, 2016

Conversation

koppor
Copy link
Member

@koppor koppor commented Sep 21, 2016

Following the discussion at #1913, that PR should be reverted.

This PR reverts the @Deprecations, but tries to keep the other improvements.

I'm not sure whether BibEntry.getResolvedField should be kept.

…1913)"

This reverts commit a9eb978.

# Conflicts:
#	src/main/java/net/sf/jabref/logic/importer/fileformat/PdfContentImporter.java
#	src/main/java/net/sf/jabref/model/database/BibDatabase.java
#	src/main/java/net/sf/jabref/model/entry/BibEntry.java
#	src/test/java/net/sf/jabref/logic/search/DatabaseSearcherTest.java
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 21, 2016
@koppor koppor added the type: code-quality Issues related to code or architecture decisions label Sep 21, 2016
@tobiasdiez
Copy link
Member

The insertEntryWithDuplicationCheck vs insertEntry can be reverted. That was my bad not to recognize that "checkDuplicaiton" also changes the bibtex key (we should rename the method!).

As for reverting the other changes, I put my veto in. Moving getResolvedField to the entry was just a refactoring without any negative side-effects (that I'm aware of). And I already commented why the static fields in BibEntry should be private: they are implementation details!

@Siedlerchr
Copy link
Member

Siedlerchr commented Sep 21, 2016

As I noted in the other issue, the problem with the static fields is that
they are used in fetchers and importers/exporters to just check if the
actual encountered field is the key or not. You can not simply set them to
private without having a solution for the things.

@tobiasdiez
Copy link
Member

Yes I know, if I would have an easy solution for this case, then I already could have implemented it and there would be no need to mark it as deprecated. Again, deprecated does not mean that it is forbidden to be used and that we have to remove it immediately but it signals "hey deer (sic!) developer, please think twice about using this method. We try to remove it but right now are not able to. Please don't make it harder for us by adding yet another use case. But go ahead if there is no other solution.".
I think this is exactly what we need for the citekey / id.

@koppor
Copy link
Member Author

koppor commented Sep 22, 2016

OK, guys. Regarding some comments at #1913 (comment), I currently cannot really judge what's the best way. I had some personal discussions and thus, I merge this in and we should discuss in the devcall how to proceed.

@koppor koppor merged commit 41dcd57 into master Sep 22, 2016
@koppor koppor deleted the revert1913 branch September 22, 2016 05:34
zesaro pushed a commit to zesaro/jabref that referenced this pull request Nov 22, 2016
…abRef#1913)" (JabRef#2023)

This reverts commit a9eb978.

# Conflicts:
#	src/main/java/net/sf/jabref/logic/importer/fileformat/PdfContentImporter.java
#	src/main/java/net/sf/jabref/model/database/BibDatabase.java
#	src/main/java/net/sf/jabref/model/entry/BibEntry.java
#	src/test/java/net/sf/jabref/logic/search/DatabaseSearcherTest.java
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 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