I agree that consensus should be reached in all changes to the system. What is not clear to me is what is the subject of consensus in this case.
As for FLINK-2423, this is clearly an issue, and the only question here is whether my solution solves it or not. I think it is fair to say that this is a trivial fix for this issue, but in any case it should be tested. I had two options: fix it without a test, fix it and add a test. Unfortunately I did not have time to add a test because I was busy with other things but I did not want to leave that bug in. We can revert the commit back which will still not give me time to write the test so the bug will potentially remain there for long (as Robert indicated he doesnt have time for it either). As for FLINK-2419, I agree that I could have added a simple test which would not have taken me long. The only reason I did this because I am testing this functionality in another PR. I understand if you want to revert this I can open a PR with the simple test added. Would it have been better if I did not address the first issue if I dont have a time to write a proper test? Then that issue would have been lingering there in a core functionality for who knows how long. I would like to clearly understand what is expected in this situation. Gyula Kostas Tzoumas <ktzou...@apache.org> ezt írta (időpont: 2015. júl. 28., K, 20:48): > I am not familiar with this part of the code, but this is perhaps a good > thing, as this is a matter of policy, not who introduced which bug (I > suspect that the policy issue was Robert's motivation for starting a thread > at the dev list) > > So, I think we have two issues: > > (1) Pull request https://github.com/apache/flink/pull/895 was merged > without addressing Gyula's comment. > > (2) Commit > > https://github.com/apache/flink/commit/78fd2146dd00da1130910d9f23f09e2504854ef7 > was > merged but consensus was not reached. > > Let's keep the two issues separate, as tracing back whose bug a PR is > fixing (recursively :-) will not lead anywhere. > > Now, back to the original question: I think that commits should be subject > to consensus in a similar way as PRs. The right to commit does not mean > that consensus should not be reached, and this is a clear case of not > having consensus. > > Kostas > > > On Tue, Jul 28, 2015 at 8:30 PM, Gyula Fóra <gyula.f...@gmail.com> wrote: > > > What concerns me here is that for FLINK-2419 I clearly indicated that > there > > is a test in my other PR, and since the fix was actually trivial, which > > didn't break the current functionality according my test, I wanted to > push > > it in before my PR because that is pending on something else. I could > have > > added a test here that is true. > > > > With FLINK-2423 I was fixing some else's mistake who disregarded my > message > > when merging a PR. We could now revert that PR that introduced that bug, > > but instead we are reverting my fix for that mistake. > > > > > > > > > > Gyula Fóra <gyula.f...@gmail.com> ezt írta (időpont: 2015. júl. 28., K, > > 20:19): > > > > > Hey, > > > > > > I am sorry that you feel bad about this, I only did not add a test case > > > for FLINK-2419 because I am adding a test in my upcoming PR which > > > verified the behaviour. > > > > > > As for FLINK-2423, it is actually very bad that issue is still there. > You > > > introduced this in your PR https://github.com/apache/flink/pull/895 > > which > > > I commented but no one fixed it before merging. As developing a test > > takes > > > quite much time here as it is tricky, I wanted to push the fix, which > was > > > in fact trivial. > > > > > > Regards, > > > Gyula > > > > > > Robert Metzger <rmetz...@apache.org> ezt írta (időpont: 2015. júl. > 28., > > > K, 20:01): > > > > > >> Hi, > > >> > > >> I'm a bit unhappy how we were handling > > >> https://issues.apache.org/jira/browse/FLINK-2419 today. > > >> > > >> I raised a concern in the JIRA because the commit for the fix didn't > > >> contain any tests. Our coding guidelines [1] imply that every feature > > >> should have tests. Apparently there were not enough tests for the two > > bugs > > >> fixed with commit 78fd2146dd. > > >> > > >> Also, Gyula's answer sounds like he is not willing to add tests right > > now. > > >> > > >> I can not remember if we ever reverted a commit in the Flink > community, > > >> but > > >> in my understanding, this is how ASF projects are doing lazy consensus > > for > > >> commits-without-PR. > > >> So if there is a disagreement in the associated JIRA, we revert the > fix > > >> until there is an agreement. > > >> > > >> In this case, I did not immediately revert the commit, because I would > > >> like > > >> to see whether others in the community agree with me. > > >> > > >> > > >> What do you think how we should handle cases like this one in the > > future? > > >> > > >> I think its very important for committers and PMC members to be a good > > >> example when it comes to following our own rules. Otherwise, how can > we > > >> ask > > >> our contributors to adhere to these rules? > > >> > > >> > > >> My suggestion to resolve this situation is the following: > > >> - Revert commit 78fd2146dd > > >> - open pull requests for FLINK-2419 and FLINK-2423 (with tests of > > course), > > >> review and merge them. > > >> > > >> > > >> > > >> Best, > > >> Robert > > >> > > >> [1] http://flink.apache.org/coding-guidelines.html > > >> > > > > > >
