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