add com.janeluo.ikkanalyzer dependency to core model#2206
add com.janeluo.ikkanalyzer dependency to core model#2206imbajin merged 5 commits intoapache:masterfrom KeeProMise:add_dependency
Conversation
|
Yep, because it's under the GPL license which is not compatible with But we do lack the doc to tell the users (so as the mysql dependency - refer #2175 (comment)) , could add some doc & comments in the config template together And maybe the example should also remove the dependency? @zyxxoo @javeme (also we should enhance the doc for |
Codecov Report
@@ Coverage Diff @@
## master #2206 +/- ##
============================================
+ Coverage 61.53% 65.04% +3.51%
- Complexity 484 979 +495
============================================
Files 497 498 +1
Lines 40572 40681 +109
Branches 5663 5681 +18
============================================
+ Hits 24965 26461 +1496
+ Misses 13061 11595 -1466
- Partials 2546 2625 +79 see 31 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Is it possible to provide a readme in the module example, explaining the dependencies |
Yep, should add the doc/comments in these spaces:
|
|
Same question, hi, @imbajin
The ik tokenizer we used seems to be this one: https://mvnrepository.com/artifact/com.janeluo/ikanalyzer/2012_u6 rather than https://github.com/blueshen/ik-analyzer The license from mvn website, does it meet the requirements? Also this one works fine by installing in local repository. |
Thanks for the reminder, I thinks it's fine to use it (but not sure) @javeme @zyxxoo Is it possible to adopt other implements? Consider replacing it |
|
I think the ikanalyzer license is Apache compatible, but not sure the reason we removed ikanalyzer from #2100 |
|
OK, It seems that we can now use this dependency. @KeeProMise @liuxiaocs7 |
I think that since ikanalyzer can be used in the core module, it can also be used in the example module. |
yep, just fix the dependence check (refer here, then we could merge it back)
And we could also add the dependence doc in the PR template or Wiki? (for others to know it) |
|
com.janeluo.ikkanalyze、lucene-queries-4.7.2.jar、lucene-queryparser-4.7.2.jar 、lucene-sandbox-4.7.2.jar is Apache 2.0, so I added the above-mentioned third-party dependencies in the known-dependencies.txt file, and added com.janeluo.ikkanalyze to the core module |
There was a problem hiding this comment.
Thanks for the contribution, and if we add a new dependency in project, we need also update the info in LICENSE & NOTICE file both in the root & dist modules (depend on source or binary dependency)
@simon824 remove provided option, shall we add/update info in dist (LICENSE & NOTICE) file (seems yes?)
And we need a doc to help new devs know how to add/update the info
|
Hi @KeeProMise ,
Update: refer with example |
Thank you for your guidance. I'll do it |
…y in LICENSE info
|
The third-party dependency does not have a NOTICE file, so I took these three steps:
|
There was a problem hiding this comment.
@simon824 can we ignore the standard Apache 2.0 license file? (seems they are duplicate?)
imbajin
left a comment
There was a problem hiding this comment.
@simon824 address it later https://github.com/apache/incubator-hugegraph/pull/2206/files#r1221441232
merge this PR first



When I run the code in the example to learn the database, I find that the class org/wltea/analyzer/core/IKSegmenter cannot be found. The scope of this class in the module core is provided, and the module example depends on the module core.

