Mailing-List: contact dev-help@flink.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@flink.apache.org
Received-SPF: error (nike.apache.org: local policy)
MIME-Version: 1.0
In-Reply-To: 
 <CALuGr6YCebbmJRxw8QU__YBm0O6JKmnkhcT6nLi8Rzc=_Zahng@mail.gmail.com>
References: 
 <CALuGr6aVrUfbVA+HkC3vizhh6nRv-cqPgR5bBo6Zjxfuy0D-3w@mail.gmail.com>
 <CANC1h_uTfHU71jwvKC5aEtg+5ZTbdtyHnOcAqEsvjdDnXZPGfg@mail.gmail.com>
 <CALuGr6YCebbmJRxw8QU__YBm0O6JKmnkhcT6nLi8Rzc=_Zahng@mail.gmail.com>
From: Max Michels <max@data-artisans.com>
Date: Thu, 5 Feb 2015 17:26:31 +0100
Message-ID: 
 <CAGco--YE=QfZNVSKUL4LVEDjrHNkJ3TnNmp14nvFhRD3DfXmbg@mail.gmail.com>
Subject: Re: [DISCUSS] Be more patient with PR and patches in the review
To: dev@flink.apache.org
Content-Type: text/plain; charset=UTF-8

Hi Henry,

I forgot to leave a message stating that I'm fine with Stephan's
changes that would soon be merged into the master. Stephan did not
push to the master immediately, so further comments could have been
made to the pull request.

It would have been more transparent if we had posted the relevant
commit. I actually just looked it up in his branch.

By the way, I absorbed Till and your feedback and will seriously
consider using alternatives to the null value.

Best regards,
Max

On Thu, Feb 5, 2015 at 5:18 PM, Henry Saputra <henry.saputra@gmail.com> wrote:
> Ah awesome, I do not about that, thanks for letting me know. Mea culpa from me.
>
> I think I saw only couple cases but thought I raise the discussions
> before I forgot =P
>
> Thanks for addressing this so quickly, Stephan.
>
> - Henry
>
>
> On Thu, Feb 5, 2015 at 8:09 AM, Stephan Ewen <sewen@apache.org> wrote:
>> Hey Henry!
>>
>> For pull request 344, I merged it, because I had already built a fix on top
>> of it while discussion was going on.
>>
>> Here is the commit that addresses actually all comments in the discussion
>> (plus a bit more)
>> https://github.com/apache/flink/commit/56b7f85b4f6d522765df19a9710a098092ccde56
>>
>> It is applied two commits later than the pull request commit.
>> It is true that I forgot to mirror that back into teh discussion. My bad!
>>
>> If you think that is happening for more pull requests, then please raise
>> the issue, because that certainly should not happen.
>>
>> Greetings,
>> Stephan
>>
>>
>> On Thu, Feb 5, 2015 at 5:03 PM, Henry Saputra <henry.saputra@gmail.com>
>> wrote:
>>
>>> HI All,
>>>
>>> I'd like to bring up a bit concerning flow I am start seeing in the few
>>> PRs.
>>>
>>> I see some PRs had been rush to commit without addressing ALL comments
>>> in the PR review.
>>> For latest example is the comments Till and I made about using Option
>>> instead of null [1] for Max's PR.
>>> It is responsibility of the PR creator to address comment raise up in
>>> the PR before any commiter could merge it. No need to rush it.
>>>
>>> Would like to see this more to make sure PRs' issue or concerns are
>>> addressed.
>>>
>>> Thanks,
>>>
>>> - Henry
>>>
>>> [1]  https://github.com/apache/flink/pull/344
>>>