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

Add Collection of Comp Sci Bibliographies fetcher #6664

Merged
merged 18 commits into from
Jul 8, 2020
Merged

Add Collection of Comp Sci Bibliographies fetcher #6664

merged 18 commits into from
Jul 8, 2020

Conversation

daniel-price
Copy link
Contributor

@daniel-price daniel-price commented Jul 1, 2020

Fixes #6638

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for working on this. It is a tough task since JabRef focuses on high code quality AND on high quality of BibTeX entries (as much as work should be done by JabRef automatically; the user should not be forced to spend any time to improve the quality of the BibTeX entries).

Thus, I gave two kinds of comments:

  • nitpicks to the code style
  • questions to the fetcher logic. Especially, because a) not all fields available are returned and b) dblp offers a BibTeX result (and it was not clear to my why this result is not used).

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

I like the added test.

Unfortunately, it shows that the abstract has some unnecessary whitespace. Could you maybe adress this?

…mputerScienceBibliographiesParserTest.java

Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

That was fast - thank you. Little nitpick remaining 😇

Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

LGTM 👍. The changes added require second review and thus this PR awaits a second review.

@Siedlerchr Siedlerchr merged commit 558c810 into JabRef:master Jul 8, 2020
@Siedlerchr
Copy link
Member

Thanks for your contribution! It would be nice if you could add some short documentation for the new formatters and the new fetcher to the help pages

@daniel-price
Copy link
Contributor Author

Yep I'll add to the documentation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New fetcher: "Collection of Computer Science Bibliographies"
3 participants