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 f
I'm probably lacking a bit of context, but by reading your conversation at
JIRA it seems to me that commit
https://github.com/apache/flink/commit/78fd2146dd00da1130910d9f23f09e2504854ef7
does not contain a test, and Robert is asking for a test which means that
we do not have consensus. If this wa
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/fli
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
a
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://gith
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
On Tue, Jul 28, 2015 at 9:09 PM, Gyula Fóra wrote:
> I agree that consensus should be reached in all changes to the system.
>
>
Then Robert and you should reach consensus on FLINK-2419.
> What is not clear to me is what is the subject of consensus in this case.
>
> As for FLINK-2423, this is cl
Hey,
I think there is no reason for making a more serious issue out of this than
it already is :)
I have opened a pull request that adds the missing test for FLINK-2419:
https://github.com/apache/flink/pull/947
There everyone can verify that my commit has actually fixed the problem.
This should h
Sounds reasonable to me.
In addition to the test-inclusion guideline, we should also pay more
attention to our single-change-per-PR rule (or commit in case of push
without PR) to avoid discussions about unrelated changes in the future.
Cheers, Fabian
2015-07-28 22:44 GMT+02:00 Gyula Fóra :
> H
I think no one here is adamant about reverting anything for the sake of
reverting. Adding the test now is just fine.
The issue was declaring something as fixed and pushing the responsibility
for testing that away. I agree with Robert that this is not the type of
example by which we should lead the
10 matches
Mail list logo
