JingGe commented on pull request #19145: URL: https://github.com/apache/flink/pull/19145#issuecomment-1073833418
> I don't mind too much one way or another. I think it was package-private as it's more narrow scope compared to `protected`, and the smaller the scope, usually the better. But IMO it doesn't matter here too much. Yeah, I had the same thought. But after I checked the code, it is defined as package-private but used as protected. In this case, it even enlarge the visibility scope, because classes within this package but do not inherit `StreamTask` are able to access it. I guess it is not intended to work as package-private and is against OO design and might cause potential issue. Another issue is that any subclass of `StreamTask` that is not in the same package will not be able to access this instance variable. If we want to limit all subclasses of `StreamTask` be in the same package, we should control it via ArchUnit rule. -- 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]
