Skip to content

Implement edge ngram tokenizer for Bloodhound#93

Merged
core-system-dev merged 1 commit into
corejavascript:masterfrom
rich186:ngram-tokenizer
Nov 1, 2016
Merged

Implement edge ngram tokenizer for Bloodhound#93
core-system-dev merged 1 commit into
corejavascript:masterfrom
rich186:ngram-tokenizer

Conversation

@rich186
Copy link
Copy Markdown

@rich186 rich186 commented Oct 31, 2016

Further to issue #91 I have implemented an Ngram tokenizer for Bloodhound along with the corresponding tests. I noticed the tokenizers were not documented in doc/Bloodhound.md so I added a Tokenizers section.

Not sure what the protocol is with regards to versioning and building, but I have built and bumped the package to 0.11.2.

Add tests for ngram tokenizer
Add tokenizer examples to Bloodhound readme
Build dist
NPM & Bower version patch to 0.11.2
@jlbooker jlbooker added this to the v0.11.2 milestone Oct 31, 2016
Copy link
Copy Markdown
Contributor

@jlbooker jlbooker left a comment

Choose a reason for hiding this comment

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

Looks good to me, and thanks for the additional documentation around tokenizers. 👍

@jlbooker
Copy link
Copy Markdown
Contributor

@CoreSystemDevelopment Could you give this a review before we merge?

Copy link
Copy Markdown
Contributor

@core-system-dev core-system-dev left a comment

Choose a reason for hiding this comment

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

@jlbooker LGTM, I talked to @lenovouser today and he told me than he was busy the last few months but will check out the repo and PR's etc. this week 👍

@core-system-dev core-system-dev merged commit cee60a9 into corejavascript:master Nov 1, 2016
@jlbooker
Copy link
Copy Markdown
Contributor

jlbooker commented Nov 1, 2016

Thanks for the review and for the update @CoreSystemDevelopment

@rich186 Thanks for the PR! 👍 We'll try to have this out in a release as soon as we can, but it may be a while. See #55

@rich186
Copy link
Copy Markdown
Author

rich186 commented Nov 1, 2016

@jlbooker Ok sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants