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

Adds the easyrdf library into the dependencies and uses it #115

Merged
merged 1 commit into from
Aug 5, 2016

Conversation

shinobu
Copy link
Contributor

@shinobu shinobu commented Jul 1, 2016

Adds the easyrdf library into the dependencies and uses it to parse rdfa in the RdfaWrapper (the rdfa import through arc2 did not work)
furthermore the RdfaWrapper got actualized and does now handle links
like the LinkedDataWrapper handled them.

@pfrischmuth pfrischmuth self-assigned this Jul 4, 2016
@pfrischmuth pfrischmuth added this to the v1.8 milestone Jul 4, 2016
@shinobu shinobu changed the title This commit added the library into the dependencies and uses it to This commit added the easyrdf library into the dependencies and uses it to Jul 4, 2016
@pfrischmuth
Copy link
Contributor

pfrischmuth commented Jul 29, 2016

  • check unit tests:
    • develop Branch: Tests: 648, Assertions: 1294, Incomplete: 3, Skipped: 8.
    • Pull Request: Tests: 648, Assertions: 1294, Incomplete: 3, Skipped: 8.

@shinobu Please add a unit test for the enhanced RdfaWrapper. You can have a look at the unit test for the LinkedDataWrapper.

@shinobu
Copy link
Contributor Author

shinobu commented Jul 30, 2016

Added a phpunit test for the RdfaWrapper

@pfrischmuth pfrischmuth changed the title This commit added the easyrdf library into the dependencies and uses it to Adds the easyrdf library into the dependencies and uses it Aug 5, 2016
@pfrischmuth
Copy link
Contributor

pfrischmuth commented Aug 5, 2016

  • manual review commits: FAILED
    • Please fix the style issues below
  • check unit tests:
    • develop Branch: Tests: 655, Assertions: 1332, Incomplete: 3, Skipped: 1.
    • Pull Request: Tests: 672, Assertions: 1350, Incomplete: 3, Skipped: 1.
    • 17 new tests with 18 new assertions
  • check integration tests mysql:
    • develop Branch: Tests: 63, Assertions: 5834, Skipped: 7.
    • Pull Request: Tests: 63, Assertions: 5834, Skipped: 7.
  • check integration tests virtuoso:
    • develop Branch: Tests: 63, Assertions: 224, Skipped: 9.
    • Pull Request: Tests: 63, Assertions: 224, Skipped: 9.
  • further manual tests:
    • consider: were unit/integration tests created that test the feature/fix? YES
    • manually tested the import of an RDFa resource in OntoWiki, which worked

}
}

private function _getHttpClient ($uri, $options = array())
Copy link
Contributor

Choose a reason for hiding this comment

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

style: remove space

@pfrischmuth
Copy link
Contributor

OK, but some style issues still need to be resolved prior merging.

@shinobu
Copy link
Contributor Author

shinobu commented Aug 5, 2016

isn't the second style issue actually correct? it's a newline prior to a new function

@pfrischmuth
Copy link
Contributor

I was relating to the space after the method name.

@shinobu
Copy link
Contributor Author

shinobu commented Aug 5, 2016

done

if those 2 style-rules are important we should add them to our codesniffer tests

@pfrischmuth
Copy link
Contributor

No, the spaces after the method names are still there.

parse rdfa in the RdfaWrapper (the rdfa import through arc2 did not work)
furthermore the RdfaWrapper got actualized and does now handle links
like the LinkedDataWrapper handled them, adds a test for the RdfaWrapper
 as well(again similiar to the LinkedDataWrapper)
@shinobu
Copy link
Contributor Author

shinobu commented Aug 5, 2016

it should be done now

@pfrischmuth
Copy link
Contributor

  • manual review commits: OK

@pfrischmuth
Copy link
Contributor

OK

@pfrischmuth pfrischmuth merged commit 8340230 into develop Aug 5, 2016
@pfrischmuth pfrischmuth deleted the fix/RdfaWrapper branch August 5, 2016 14:25
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.

2 participants