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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> >>> 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 >>>>
