flink-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Henry Saputra <henry.sapu...@gmail.com>
Subject Re: [DISCUSS] Be more patient with PR and patches in the review
Date Thu, 05 Feb 2015 16:29:47 GMT
Thanks Max, really appreciate it.

Stephan just pulled me aside and tell me that he had merge the changes
into separate branch.
Bit misunderstanding from my side, but I want to make sure Flink
community be awesome and great place for people to come in and
contribute.

Apologize for jumping the gun there.

- Henry

On Thu, Feb 5, 2015 at 8:26 AM, Max Michels <max@data-artisans.com> wrote:
> 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
>>>>

Mime
View raw message