magibney commented on pull request #15: URL: https://github.com/apache/lucene/pull/15#issuecomment-806055837
Great! yes this is looking pretty good from my perspective too. (I forgot to address the `hoistFilter` method rename -- thanks for catching that). I'm curious what you'll make of 0d8c001 ... the previous state was kind of weird because we were ostensibly "detecting" norm ids that never actually cropped up in practice, but then throwing an `UnsupportedOperationException` if we had ever come to the point of trying to replace them. This worked because of the fact that they never cropped up in practice. I'm pretty sure that the change introduced in 0d8c001 would work fine, but at the moment it's definitely not covered by tests. Alternatives to 0d8c001 would be: 1. stop detecting the strings FCC, FCD, and NFKC_CF (i.e. don't recognize them as candidates for replacement/optimization) 2. continue detecting FCC/FCD/NFKC_CF and leave in place the new code that purports to handle them, but `assert` (either in a test over `availableIDs`, or in live code so that the check would be a noop unless assertions were enabled) that we should never actually _encounter_ these types of leading/trailing normalization. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
