Exception shouldn't happen when pasting an entry with a publication date-range of form yyyy-yyyy#8247
Merged
Merged
Conversation
Siedlerchr
reviewed
Nov 19, 2021
Siedlerchr
reviewed
Nov 19, 2021
Siedlerchr
requested changes
Nov 19, 2021
Siedlerchr
left a comment
Member
There was a problem hiding this comment.
Can you please write a test for this? And also what happens to some alias fields like month etc?
When I see this right, you just save the second date, but otherwise it's ignored?
| * | ||
| * @param date the start date | ||
| * @param endDate the start date | ||
| * @throws NullPointerException if DOI/Short DOI is null |
Member
There was a problem hiding this comment.
DOI? Seems like you copied the wrong comments
Contributor
Author
There was a problem hiding this comment.
- My bad, yes, I copied the wrong java comments.
- Do you have any suggestions where to put the JUnit test, or should I create a new .java file for this test?
- "And also what happens to some alias fields like month etc?
When I see this right, you just save the second date, but otherwise it's ignored?"
if it is the format mm/uuuu, it is still parsed as month/year
if it is the format uuuu/uuuu, it is parsed as year range
Your understanding is 100% correct.
Thanks so much for the detailed review.
Member
There was a problem hiding this comment.
- In this case, the easiest is to search for the file
DateTest(located insrc/test/java/org/jabref/model/entry/DateTest.java). If you ever need to create a completely new test file I'd suggest the IntelliJ short-cut, the default option follows the same naming convenient that we are using. - Not sure that I understand, but I am guessing there is no question here?
There was a problem hiding this comment.
There's also some more information on the test conventions we are following in https://devdocs.jabref.org/getting-into-the-code/code-howtos#test-cases
Contributor
Author
There was a problem hiding this comment.
thanks very much for the suggestions.
Contributor
Author
There was a problem hiding this comment.
I removed the improper comments added the unit test.
I really appreciate all the constructive and detailed feedback. I felt very lucky to choose JabRef as my first open-source project. Everyone is so nice and helpful.
Contributor
Author
|
I added the requested Junit test for the modified parse function in Date.java, and please let me know if there is anything else I should change. |
Siedlerchr
reviewed
Dec 8, 2021
calixtus
approved these changes
Dec 14, 2021
Siedlerchr
added a commit
that referenced
this pull request
Dec 17, 2021
* upstream/main: Don't register any database changes to the indexer while dropping a file (#8334) Fix ACM fetcher (#8338) Squashed 'buildres/csl/csl-styles/' changes from 3bb4b5f..60bf7d5 Exception shouldn't happen when pasting an entry with a publication date-range of form yyyy-yyyy (#8247) Refactor Sidepane logic (#8202) New translations JabRef_en.properties (Japanese) (#8331) Bump bcprov-jdk15on from 1.69 to 1.70 (#8333) Update Controlsfx to 11.1.1 (#8330) Update citeproc (#8329) Bump classgraph from 4.8.137 to 4.8.138 (#8327) Bump junit-platform-launcher from 1.8.1 to 1.8.2 (#8326) Remove outdated gradle deps check (#8324) Bump junit-jupiter from 5.8.1 to 5.8.2 (#8325) Bump libreoffice from 7.2.2 to 7.2.3 (#8328) Remove outdated ignores
Siedlerchr
added a commit
that referenced
this pull request
Dec 20, 2021
* upstream/main: (46 commits) New Crowdin updates (#8349) Bump pdfbox from 2.0.24 to 2.0.25 (#8345) Bump fontbox from 2.0.24 to 2.0.25 (#8348) Bump xmlunit-core from 2.8.3 to 2.8.4 (#8347) Bump tika-core from 2.1.0 to 2.2.0 (#8346) Added missing executable bindings to various commands (#8342) Update Gradle Wrapper from 7.3.1 to 7.3.2. (#8343) Fix issues with writing metadata to pdfs (#8332) add tinylog test (#8339) Tinylog (#8226) Don't register any database changes to the indexer while dropping a file (#8334) Fix ACM fetcher (#8338) Squashed 'buildres/csl/csl-styles/' changes from 3bb4b5f..60bf7d5 Exception shouldn't happen when pasting an entry with a publication date-range of form yyyy-yyyy (#8247) Refactor Sidepane logic (#8202) New translations JabRef_en.properties (Japanese) (#8331) Bump bcprov-jdk15on from 1.69 to 1.70 (#8333) Update Controlsfx to 11.1.1 (#8330) Update citeproc (#8329) Bump classgraph from 4.8.137 to 4.8.138 (#8327) ... # Conflicts: # build.gradle
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes: #7864
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)