Robert Muir

Results 269 comments of Robert Muir

I looked, I don't think the normalization optimizations here are safe. Many rulesets assume a certain form for a reason, because the rules work that way. For example, Hangul-Latin works...

I think it would be best if we removed it from this PR and look at it as a followup. There are other alternatives that can be done in a...

> My conception of this arg is as an "expert" option that can significantly improve performance, but requires users to know (and externally compensate for, with specific ICUNormalizer2CharFilters in the...

@magibney I think this is really close to being merged? When testing against current `main`, the new static analysis checks complain about a piece of dead code in the Factory...

> I'm curious what you'll make of [0d8c001](https://github.com/apache/lucene/commit/0d8c001d88bda373fe321550a452c4dd53a3af74) ... the previous state was kind of weird because we were ostensibly "detecting" norm ids that never actually cropped up in practice,...

Thank you, ICU side looks good! Let's just take care of BaseTokenStreamTestCase and then I think we are ready to merge. I'm sorry this PR has taken so embarrassingly long....

I got the precommit "working" by just disabling a bunch of build checks with corresponding `TODO` in the source code, reducing visibility of some stuff that didn't need to be...

ugh, and i guess that `spotlessApply` really made some of the code ugly, especially comments. maybe we can manually wrap them in a way that the spotless checker still accepts....

yes, it is much easier for me to help out if the build and tests are working, I can't really review otherwise because I rarely write java these days. So...

the tests are failing for me locally too. Mostly it seemed to be previous implementations test? It does `assertEquals(AnalysisResult a, AnalysisResult b)` but AnalysisResult has no equals()...