Thanks very much Yang Wang. Cheers, Canbin Zheng
Yang Wang <danrtsey...@gmail.com> 于2020年2月27日周四 下午9:19写道： > Great work! I could help to review and test. > > Best, > Yang > > Canbin Zheng <felixzhen...@gmail.com> 于2020年2月27日周四 下午4:24写道： > >> 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. >>>>>>>>> >>>>>>>>
