Skip to content

Conversation

@thygesen
Copy link
Contributor

Here is a test for the BilouNameFinderSequenceValidator. It proved to have bugs according to my tests and I therefore also fixed NameFinderSequenceValidator. Should this have been a separate PR/JIRA?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 55.043% when pulling d59e825 on thygesen:opennlp-1000 into a9cfd7e on apache:master.

@kottmann
Copy link
Member

The changes look good, but we still need to update the commit message before merging this.

Copy link
Member

@kottmann kottmann left a comment

Choose a reason for hiding this comment

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

+1 lgtm

@kottmann
Copy link
Member

kottmann commented Mar 13, 2017

We need to run the validation tests once with those fixes applied and if not present add some.

Copy link
Member

@wcolen wcolen left a comment

Choose a reason for hiding this comment

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

+1 lgtm (reduce the commit message length)

@thygesen
Copy link
Contributor Author

pls. let me know if you need me to do something.

@kottmann
Copy link
Member

No, this is good, I just need to run the tests and I will also rewrite the commit message (should contain the issue)

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.

4 participants