Refactor EprintCleanup to handle institution, version, and EID fields#11627
Conversation
koppor
left a comment
There was a problem hiding this comment.
Small comment.
Moreover, please add the link to the documentation of the identifier, Tobias provided. That knowledge should be persistet in the code!
| @Test | ||
| void cleanupEntryWithVersionAndInstitutionAndEid() { | ||
| BibEntry input = new BibEntry() | ||
| .withField(StandardField.NOTE, "arXiv: 1503.05173") | ||
| .withField(StandardField.VERSION, "1") | ||
| .withField(StandardField.INSTITUTION, "arXiv") | ||
| .withField(StandardField.EID, "arXiv:1503.05173"); | ||
|
|
||
| BibEntry expected = new BibEntry() | ||
| .withField(StandardField.EPRINT, "1503.05173v1") | ||
| .withField(StandardField.EPRINTTYPE, "arxiv"); | ||
|
|
||
| EprintCleanup cleanup = new EprintCleanup(); | ||
| cleanup.cleanup(input); | ||
|
|
||
| assertEquals(expected, input); | ||
| } |
There was a problem hiding this comment.
Please add another test with INSTITUTION tbd. That field value should be kept IMHO. --> JabRef should not destory data.
There was a problem hiding this comment.
@koppor I have made the suggested changes and pushed it to this PR , please review.
There was a problem hiding this comment.
I think, I was not clear with another
This means: A second test case. This means: Keep the original test as is.
I did not also not "unzip" my comment, sorry!
This will require changes in the code. The code needs to check if the institution is arXiv. Then, the field can be cleared. Otherwise, the field needs to be kept.
| @Test | ||
| void cleanupEntryWithVersionAndInstitutionAndEid() { | ||
| BibEntry input = new BibEntry() | ||
| .withField(StandardField.NOTE, "arXiv: 1503.05173") | ||
| .withField(StandardField.VERSION, "1") | ||
| .withField(StandardField.INSTITUTION, "arXiv") | ||
| .withField(StandardField.EID, "arXiv:1503.05173"); | ||
|
|
||
| BibEntry expected = new BibEntry() | ||
| .withField(StandardField.EPRINT, "1503.05173v1") | ||
| .withField(StandardField.EPRINTTYPE, "arxiv"); | ||
|
|
||
| EprintCleanup cleanup = new EprintCleanup(); | ||
| cleanup.cleanup(input); | ||
|
|
||
| assertEquals(expected, input); | ||
| } |
There was a problem hiding this comment.
I think, I was not clear with another
This means: A second test case. This means: Keep the original test as is.
I did not also not "unzip" my comment, sorry!
This will require changes in the code. The code needs to check if the institution is arXiv. Then, the field can be cleared. Otherwise, the field needs to be kept.
| BibEntry expected = new BibEntry() | ||
| .withField(StandardField.EPRINT, "1503.05173v1") | ||
| .withField(StandardField.EPRINTTYPE, "arxiv") | ||
| .withField(StandardField.INSTITUTION, "tbd"); |
There was a problem hiding this comment.
Lets use other-institution as example for the new test.
There was a problem hiding this comment.
@koppor I have done the necessary changes, updated the EprintCleanup class to clear the institution field if it is "arXiv" else keep it otherInstitution as it is.
|
@18bce133 Hey, how is it going? Are you still working on this PR? |
|
Hey @subhramit ! I'm still on the PR. I've been quite busy lately with some interviews lined up, which slowed me down a bit. I'll make sure to wrap up this PR in the next few days. Thanks for checking in! |
There was a problem hiding this comment.
Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.
You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".
|
CHANGELOG.md was not modified. I add the entry by myself to get this merged. |
What
This PR refactors the EprintCleanup class to handle additional fields: INSTITUTION, VERSION, and EID. It also includes new tests to ensure the correct functionality of these changes.
Why
The changes are necessary to improve the handling of e-print entries by including more comprehensive cleanup logic. This ensures that entries with INSTITUTION, VERSION, and EID fields are properly normalized and cleaned up.
Changes
Issue
Closes #11306
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)