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

Fix metadata extractors #72

Merged
merged 8 commits into from Feb 18, 2017
Merged

Fix metadata extractors #72

merged 8 commits into from Feb 18, 2017

Conversation

@c-w
Copy link
Owner

@c-w c-w commented Feb 18, 2017

  • Add more data for metadata querying tests
  • Fix test setup

This PR resolves #70.

c-w added 7 commits Feb 18, 2017
The unit tests related to "subject" metadata were set up incorrectly
(invalid N-Triples formatting of the test data). This was not caught
before because there was no test data exercising this code path.
The simple predicate extractor needs a contains method to do its job, so
might as well document this via an abstract method that subclasses need
to implement.
The unit tests related to "language" metadata were set up incorrectly
(invalid N-Triples formatting of the test data). This was not caught
before because there was no test data exercising this code path.
@coveralls
Copy link

@coveralls coveralls commented Feb 18, 2017

Coverage Status

Coverage increased (+0.5%) to 94.521% when pulling 634eaa9 on fix-metadata-extractors into bd139bb on master.

This localizes all the code for every extractor into just one place
which makes it easier to grasp all the things that the extractor does.
For example, with this structure, it's obvious that the language
extractor needs to bind the RFC 4646 type to string.
@coveralls
Copy link

@coveralls coveralls commented Feb 18, 2017

Coverage Status

Coverage increased (+0.5%) to 94.545% when pulling c8d4101 on fix-metadata-extractors into bd139bb on master.

@c-w c-w merged commit 042ae21 into master Feb 18, 2017
3 checks passed
@c-w c-w deleted the fix-metadata-extractors branch Feb 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants