Skip to content

Check more results returned by CrossRef API for matching#2531

Merged
stefan-kolb merged 2 commits into
masterfrom
crossref-fetcher
Feb 9, 2017
Merged

Check more results returned by CrossRef API for matching#2531
stefan-kolb merged 2 commits into
masterfrom
crossref-fetcher

Conversation

@stefan-kolb

@stefan-kolb stefan-kolb commented Feb 8, 2017

Copy link
Copy Markdown
Member

Resolves #2431

Changes:

  • We will now check 5 results instead of just the first one for a matching title.

@stefan-kolb stefan-kolb added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Feb 8, 2017
@stefan-kolb stefan-kolb changed the title Resolves #2431 Check more results returned by CrossRef API for matching Check more results returned by CrossRef API for matching Feb 8, 2017
}
}
} catch(JSONException ex) {
return Optional.empty();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Mabye useful to add a logging here in case the json format changes?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be detected by our Tests. Shouldn't it?

However, logging here does no harm.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test will fail yes.

@matthiasgeiger

Copy link
Copy Markdown
Member

LGTM

Do you want to implement #2455 as well? 😻

@stefan-kolb stefan-kolb merged commit 192e4dd into master Feb 9, 2017
@stefan-kolb stefan-kolb deleted the crossref-fetcher branch February 9, 2017 09:26
@matthiasgeiger

Copy link
Copy Markdown
Member

Ok I interpret this as a "no" 😉

@stefan-kolb

Copy link
Copy Markdown
Member Author

nah will do that later, maybe at jabcon. is a separate task.

Siedlerchr added a commit that referenced this pull request Feb 9, 2017
* upstream/master:
  Use LaTeX free title for CrossRef search #1424
  Check more results returned by CrossRef API for matching (#2531)
  Rework File links logic (#2529)
  Remove MIT string from lang files
  Translate new strings (#2528)
  Make DOIFetcher the default fetcher in new entry dialog (#2530)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants