Thanks for clarifying. It seems like this is a case where there's a broader discussion to be had about the merits of the optimization (completely separate from the issue at hand).
1) I'm not opposed to developing a fix that's better than one which was already proposed although I think it would be good to run it by whoever originally filed the issue. 2) I think "backward" here is subjective. I'm not picking a side here, but certainly there are cases where disabling a buggy optimization is the right thing to do. 3) Developing a different fix may sometimes be the right thing to do, but I think other contributors would appreciate a discussion before their code is effectively ignored. -- Michael Mior mm...@apache.org Le mar. 28 août 2018 à 17:01, Vladimir Sitnikov <sitnikov.vladi...@gmail.com> a écrit : > Michael>One of the other side effects in this case > Michael>seems to be (without having examined the technical merit of either > Michael>solution) that the fix which was ultimately committed still didn't > solve > Michael>the original issue. > > I'm afraid you did are wrong here. > My first commit implemented exactly one test. I removed @Ignored from the > test and implemented a fix. > > It turned out Zoltan crafted more complex test that identified a bug in the > v1 of the implementation. > Note: that was a new test, and it was not included in PR707. > Note: there are millions of test cases missing, and I know the proper way > to cover it. > However, it looks like everybody here likes "one test per issue" approach > more, so I follow it somehow: I unlock a single test, so everybody is > happy. > > Michael>In this case, it seems like Zoltan was still willing to help > provide a > solution > > AFAIK, no-one (including Zoltan) cares to suggest a test case to defeat > current code in master. > I treat that as "the feature is good enough". > > Michael>see the last activity on both of those before the period of > inactivity was > by Zoltan > > My point here is > 1) It takes ~30 min to "develop+test" the fix > 2) PR707 goes in opposite direction: it disables the optimization instead > of just unlocking a single @Ignore test > 3) The bug does bother me > ==> I just fix it and merge it. > On top of that, I see nothing I could reuse from PR707, so I had to just > discard it. > > Vladimir >
