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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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.

koppor
koppor requested changes Jul 1, 2020
View reviewed changes
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).

CHANGELOG.md Outdated Show resolved Hide resolved
...java/org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcher.java Show resolved Hide resolved
...java/org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcher.java Outdated Show resolved Hide resolved
.../java/org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesParser.java Outdated Show resolved Hide resolved
.../java/org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesParser.java Outdated Show resolved Hide resolved
.../org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcherTest.java Outdated Show resolved Hide resolved
.../org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcherTest.java Show resolved Hide resolved
...rter/fetcher/collection_of_computer_science_bibliographies_multiple_results_first_result.bib Outdated Show resolved Hide resolved
...abref/logic/importer/fetcher/collection_of_computer_science_bibliographies_single_result.bib Outdated Show resolved Hide resolved
...abref/logic/importer/fetcher/collection_of_computer_science_bibliographies_single_result.xml Outdated Show resolved Hide resolved
daniel-price and others added 7 commits July 1, 2020 20:51
koppor
koppor requested changes Jul 7, 2020
View reviewed changes
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?

.../org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcherTest.java Outdated Show resolved Hide resolved
.../org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesFetcherTest.java Show resolved Hide resolved
...a/org/jabref/logic/importer/fetcher/CollectionOfComputerScienceBibliographiesParserTest.java Outdated Show resolved Hide resolved
...jabref/logic/importer/fetcher/collection_of_computer_science_bibliographies_empty_result.xml Show resolved Hide resolved
...abref/logic/importer/fetcher/collection_of_computer_science_bibliographies_single_result.bib Show resolved Hide resolved
@daniel-price @koppor
Update src/test/java/org/jabref/logic/importer/fetcher/CollectionOfCo…
2b2646b 
…mputerScienceBibliographiesParserTest.java

Co-authored-by: Oliver Kopp <[email protected]>
koppor
koppor requested changes Jul 7, 2020
View reviewed changes
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 😇

src/main/java/org/jabref/logic/formatter/bibtexfields/RemoveTabsFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/RemoveTabsFormatter.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/RemoveRedundantSpacesFormatter.java Show resolved Hide resolved
src/main/java/org/jabref/logic/formatter/bibtexfields/RemoveRedundantSpacesFormatter.java Outdated Show resolved Hide resolved
koppor
koppor approved these changes Jul 7, 2020
View reviewed changes
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

@koppor
Copy link
Member

koppor commented Jul 8, 2020

Help page for formatters: https://github.com/JabRef/user-documentation/blob/master/en/finding-sorting-and-cleaning-entries/cleanupentries.md --> https://github.com/JabRef/user-documentation/blob/master/en/advanced/saveactions.md

Help page for fetchers: https://github.com/JabRef/user-documentation/tree/master/en/collect/import-using-online-bibliographic-database

@daniel-price
Copy link
Contributor Author

Yep I'll add to the documentation!

@koppor koppor mentioned this pull request Jun 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@koppor koppor koppor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New fetcher: "Collection of Computer Science Bibliographies"
3 participants
@daniel-price @Siedlerchr @koppor