[GitHub] [lucene] uschindler edited a comment on pull request #740: LUCENE-10393: Unify binary dictionary and dictionary writer in kuromoji and nori

[GitBox <gi...@apache.org>]

uschindler edited a comment on pull request #740:
URL: https://github.com/apache/lucene/pull/740#issuecomment-1071008332


   I like the idea to remove the code duplication and have only one implementation.
   
   On the other hand, if you look at LOC before/after: +1,818 −1,492
   We now have 326 lines more, in addition both NORI and Kuromojo hardly depend on code in a different module (analysis-common), which now gets public.
   
   So I feel a bit ambiguous about if it really makes sense to forcefully combine those implementations just because we have an overlap of "about 200 lines" (did not count).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org