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