Hi, everyone,

I have pushed a PR <https://github.com/apache/flink/pull/11233> for this
issue, looking forward to your feedback.


Cheers,
Canbin Zheng

Canbin Zheng <felixzhen...@gmail.com> 于2020年2月26日周三 上午10:39写道:

> Thanks for the detailed PR advice, I would separate the commits as clear
> as possible to help the code reviewing.
>
>
> Cheers,
> Canbin Zheng
>
> tison <wander4...@gmail.com> 于2020年2月25日周二 下午10:11写道:
>
>> Thanks for your clarification Yang! We're on the same page.
>>
>> Best,
>> tison.
>>
>>
>> Yang Wang <danrtsey...@gmail.com> 于2020年2月25日周二 下午10:07写道:
>>
>>> Hi tison,
>>>
>>> I do not mean to keep two decorator at the same. Since the two
>>> decorators are
>>> not api compatible, it is meaningless. I am just thinking how
>>> to organize the
>>> commits/PRs to make the review easier. The reviewers may need some
>>> context
>>> to get the point.
>>>
>>>
>>>
>>> Best,
>>> Yang
>>>
>>> tison <wander4...@gmail.com> 于2020年2月25日周二 下午8:23写道:
>>>
>>>> The process in my mind is somehow like this commit[1] which belongs to
>>>> this pr[2]
>>>> that we firstly introduce the new implementation and then replace it
>>>> with the original
>>>> one. The difference is that these two versions of decorators are not
>>>> api compatible
>>>> while adding a switch for such an internal abstraction or extracting a
>>>> clumsy
>>>> "common" interface doesn't benefit.
>>>>
>>>> Best,
>>>> tison.
>>>>
>>>> [1]
>>>> https://github.com/apache/flink/commit/1f2969357c441e24b71daef83d21563da9a93bb4
>>>> [2] https://github.com/apache/flink/pull/9832
>>>>
>>>>
>>>>
>>>>
>>>> tison <wander4...@gmail.com> 于2020年2月25日周二 下午8:08写道:
>>>>
>>>>> I agree for separating commits we can have multiple commits that
>>>>> firstly add the new parameters
>>>>> and decorators,  and later replace current decorators with new
>>>>> decorators which are well
>>>>> unit tested.
>>>>>
>>>>> However, it makes no sense we have two codepaths from FlinkKubeClient
>>>>> to decorators
>>>>> since these two version of decorators are not api compatible and there
>>>>> is no reason we keep both
>>>>> of them.
>>>>>
>>>>> Best,
>>>>> tison.
>>>>>
>>>>>
>>>>> Yang Wang <danrtsey...@gmail.com> 于2020年2月25日周二 下午7:50写道:
>>>>>
>>>>>> I think if we could, splitting into as many PRs as possible is good.
>>>>>> Maybe we could
>>>>>> introduce the new designed decorators and parameter parser first, and
>>>>>> leave the existing
>>>>>> decorators as legacy. Once all the new decorators is ready and well
>>>>>> tested, we could
>>>>>> remove the legacy codes and use the new decorators in the kube client
>>>>>> implementation.
>>>>>>
>>>>>>
>>>>>> Best,
>>>>>> Yang
>>>>>>
>>>>>> Canbin Zheng <felixzhen...@gmail.com> 于2020年2月25日周二 下午6:16写道:
>>>>>>
>>>>>>> Hi, Till,
>>>>>>>
>>>>>>> Great thanks for your advice, I totally agree with you to split the
>>>>>>> changes up in as many PRs as possible. The part of "Parameter Parser" is
>>>>>>> trivial so that we prefer to make one PR to avoid adapting a lot of 
>>>>>>> pieces
>>>>>>> of code that would be deleted immediately with the following decorator
>>>>>>> refactoring PR. Actually I won't insist on one PR, could it be possible
>>>>>>> that I first try out with one PR and let the committers help assess 
>>>>>>> whether
>>>>>>> it is necessary to split the changes into several PRs?  Kindly expect to
>>>>>>> see your reply.
>>>>>>>
>>>>>>

Reply via email to