-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Implemented OpenAlex #14020
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
Implemented OpenAlex #14020
Conversation
Hey @MartinBuzogan!Thank you for contributing to JabRef! Your help is truly appreciated ❤️. We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. Please re-check our contribution guide in case of any other doubts related to our contribution workflow. |
// DOI | ||
if (item.has("publication_year")) { | ||
String doi = item.optString("doi"); | ||
doi = doi.replaceFirst("^https://doi\\.org/", ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to split the url from doi, we have cleanup/converter for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, i will remove unnecessary cleanup
@Test | ||
void parserParsesSingleWorkObject() throws Exception { | ||
Parser parser = fetcher.getParser(); | ||
String json = "{" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can convert this to multi-line string (three quotation marks) intellij should offer an action for it
Thanks a lot for the contribution, looks already mostly good. Please have a look at the failing test. |
…s to multi-line strings & applied DoiCleanup instead of removing it manualy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm, thanks!
P.S. (for future) please don't remove the steps to test section from the PR description.
After merging the main branch into mine I have issues with some of the pipeline, I don't know where is the problem before it passed all the tests |
Seems like an issue with sonatype or jitpack, resolving a snapshot dependency fails. not related to your changes |
* implementation of OpenAlexFetcher , Added UnitTest * updated changelog * updated tests for OpenAlex * Updated WebFetcher to correct SourceCode test & converted parser tests to multi-line strings & applied DoiCleanup instead of removing it manualy * OpenAlexFetcherTest Reformated * Changed annotation NotNull to NonNull
Are you interested in implementing the fetcher in #13400 as well? |
Closes #13940
Mandatory checks
CHANGELOG.md
in a way that is understandable for the average user (if change is visible to the user)