Conversation
gerardobort
left a comment
gerardobort
left a comment
There was a problem hiding this comment.
Looking good so far. Thanks for your time, and helping improve this library!
Just a recommendation: if you can, please follow https://www.conventionalcommits.org/en/v1.0.0-beta.2/ for the commit messages. Not a problem anyways... I'd squash and merge to master, making sure the final commit follows that standard. I have pending to add CONTRIBUTING.md.
| "lint": "eslint \"src/**/*.js\" \"test/*.js\"", | ||
| "lint:fix": "npm run lint -- --fix", | ||
| "test": "nyc --reporter=html --reporter=text mocha test/setup.js --sort 'src/**/*.spec.js' --compilers js:babel-core/register --timeout 30000", | ||
| "test": "nyc --reporter=html --reporter=text mocha test/setup.js --sort \"src/**/*.spec.js\" --compilers js:babel-core/register --timeout 30000", |
There was a problem hiding this comment.
please, make sure this doesn't come as new change, since it was introduced by #51 ... rebasing your branch with the latest master HEAD should do the trick
There was a problem hiding this comment.
Wow, thats great. I never had a concrete use-case for git rebase and therefore probably never really understood it. Thanks for changing that. 😄
| RegexNERAnnotator, | ||
| CorefAnnotator, | ||
| NaturalLogicAnnotator, | ||
| OpenIEAnnotator, |
There was a problem hiding this comment.
nit: please, use the same order as for the import statements (that's OCD 😆I know).
| RegexNERAnnotator, | ||
| CorefAnnotator, | ||
| NaturalLogicAnnotator, | ||
| OpenIEAnnotator, |
There was a problem hiding this comment.
nit: same here, let's follow the same order.
| regexner, | ||
| coref, | ||
| natlog, | ||
| openie, |
| * @requires tokenize, ssplit, pos, lemma, depparse (Can also use parse) | ||
| * @see {@link https://stanfordnlp.github.io/CoreNLP/natlog.html|NaturalLogicAnnotator} | ||
| */ | ||
| class NaturalLogicAnnotator extends Annotator { |
| * @requires natlog | ||
| * @see {@link https://stanfordnlp.github.io/CoreNLP/openie.html|OpenIEAnnotator} | ||
| */ | ||
| class OpenIEAnnotator extends Annotator { |
|
Thanks for the great advice! The problems you've mentioned are fixed now. I'll probably get to dive into natlog on the weekend. 🎉 |
2 participants
This aims to close #50.
Uploading this as a work in progress, since natlog is not complete, yet. @gerardobort maybe you can take a quick look and check if I am working in the right direction.