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. >>>>>>>> >>>>>>>
