reschke commented on PR #1667: URL: https://github.com/apache/jackrabbit-oak/pull/1667#issuecomment-2313014745
Yes, I do remember that case. At the end of the day, every issue can be tracked to lack of tests. In this particular case, we could of course decide not to change any test classes, because it really does not matter whether they use Guava or not. The main strategy that I used for this activity is to identify the Guava usages that are simple and mechanical to do, because we have equivalents in the JDK or common-collections. Where this is the case, we should just do the obvious translation, and be done with it. Potential optimizations (like using a simple Map instead of a BidiMap) can of course be done if we feel it's important, but it should *not* be combined with the actual Guava replacment, unless the change *really* is trivial. Anything that requires some kind of analysis is not, because we have *hundreds* of lines of code to change. WRT your example: this happened because (a) it was less than obvious that the replacement was not fully compatible, and (b), of course, lack of test coverage. I do agree that we need to be careful, and the best way to do so is to resist the temptation to micro-manage every change. So, in summary: I disagree. I'd like to hear what others think. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
