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

Follow up #2428 #2438

Merged
merged 4 commits into from
Jan 15, 2017
Merged

Follow up #2428 #2438

merged 4 commits into from
Jan 15, 2017

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Dec 28, 2016 •
edited
Loading

Follow-up from #2428:

  • Extract parsing of identifier to new class
  • Add ArXiv fetcher to Id-based fetcher (and add tests that all fetcher are contained there)
  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 28, 2016
lenhard
lenhard approved these changes Jan 1, 2017
View reviewed changes
Copy link
Member

@lenhard lenhard left a comment

Choose a reason for hiding this comment

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

Just a few minor things, otherwise the code looks good. Can you also rename the class Arxiv to ArxivFetcher (since this is the appropriate name, I guess)?

src/test/java/net/sf/jabref/gui/importer/fetcher/EntryFetchersTest.java
private Set<? extends Class<?>> getClasses(List<?> objects) {
return objects.stream().map(Object::getClass).collect(Collectors.toSet());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline at the end of the file

src/test/java/net/sf/jabref/model/entry/ArXivIdentifierTest.java

assertEquals(Optional.of(new ArXivIdentifier("0710.0994")), parsed);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Missing newline at the end of the file.

@tobiasdiez
Copy link
Member Author

Fixed newline issues but didn't renamed the fetcher since most of the fetchers don't have a "Fetcher" suffix.

@tobiasdiez tobiasdiez merged commit f57a2e0 into JabRef:master Jan 15, 2017
@tobiasdiez tobiasdiez deleted the arxiv branch January 15, 2017 12:31
Siedlerchr added a commit that referenced this pull request Jan 18, 2017
@Siedlerchr
Merge remote-tracking branch 'upstream/master' into moveFileDir
0ca9e1d 
* upstream/master:
  Save deletion of current searchquery (#2469)
  Update dev dependencies (mockito, wiremock, assertj)
  Update BouncyCastle (1.55->1.56), ANTRL4 (4.5.3->4.6), jsoup (1.10.1->1.10.2)
  Group all checker which only check the value of one field (#2437)
  Follow up #2428 (#2438)
  Fix conversion of "'n" and "\'{n}" from LaTeX to Unicode (#2464)
  Fix failing tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lenhard lenhard lenhard approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tobiasdiez @lenhard