syhily commented on pull request #16900: URL: https://github.com/apache/flink/pull/16900#issuecomment-905865179
> LGTM % suggestion. For future contribution, we need to strive for smaller self-contained commits. In this case, I can still see it as an amendment of the first initial big commit. > > So we could have 1 commit that just deals with removing `ClientConfigurationData`. Then 1 commit improves documentation. 1 commit for improved configuration validation etc. > > That makes reviewing much easier and helps future maintainers (including you) to understand why certain changes happened in the past. Thanks for this detailed instruction. I have split the second big commit into small commits. This makes me think twice before pushing the code. Why I need this modification? Is it related with current issue? I still have a long way to go on how to contribute code in the open source community. But I would try my best to learn these knowledge. -- 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]
