ferenc-csaky commented on PR #27333: URL: https://github.com/apache/flink/pull/27333#issuecomment-3656850517
@RocMarshal Thanks a lot for taking the time and effort to summarize where the original implementation comes from. Based on that, I agree that using the name `ResourceUnitCount` would be wrong if we take the original intent into account. However, my main problem here is with the name itself. `LoadingWeight` is too abstract, means and tied to pretty much nothing, so if a couple months from now somebody else are in the same shoes as me, they still need to spend significant amount of time to understand what's going on here actually. `WeightLoadable` IMO is a bit worse, cause it's a) grammatically does not represent what the interface can be used to, and b) not obvious it is connected to `LoadingWeight` (if I just bum. So based on this, I'd suggest the naming below: - `LoadingWeight` -> `SchedulingLoad` (with a `getLoad()` method) - `WeightLoadable` -> `HasSchedulingLoad` (with `getSchedulingLoad()`) I'm open to other ideas, but I do not think we should leave it as is. >I'm trying to speculate whether you simply find the usage of LoadingWeight in [2] inappropriate. If that's the case, then adopting a new set of interface abstractions to handle parameter passing and corresponding logic calculations throughout the entire chain—without altering how LoadingWeight is used in [1]—sounds like an excellent alternative. I'm not sure I get the intent completely, but I really believe we should not make this more complex for now (I really like the "you ain't gonna need it" principle, and the best code is the code that does not exist, cause that won't have bugs). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
