1996fanrui commented on PR #24003: URL: https://github.com/apache/flink/pull/24003#issuecomment-1905208562
> Fair enough. What about squashing the [FLINK-33565](https://issues.apache.org/jira/browse/FLINK-33565)-related commits into a single one? Hi @XComp , it's fine for me. Let me explain why I split them into multiple commits: 1. First of all, the doc of Flink code style introduces us that it's better to separate refactoring, cleanup and independent-changes as the separate commits. As I understand, it has a series of advantages: - Easy to review - Easy to revert - Easy to cherry-pick - In my daily work, we always cherry-pick some new features(our users needed) into our internal flink version, because a lot of flink jobs upgrade the flink version needs a lot of efforts. - During cherry-pick, I found let the ` independent-changes` as the separate commit is very useful. - For example, we need to cherry pick featureB, and featureA is merged before featureB. - FeatureB was using some common changes of FeatureA, FeatureA added some new fields in some common classes.(It's not refactor, but the change isn't huge.) - If FeatureA think it's minor change and keep all changed into 1 commit. We must cherry-pick featureA during cherry-pick featureB. - If featureA split them into multiple commits for each independent-changes, we can cherry-pick the common class changes of featureA, and the whole featureB. 2. Currently, the first commit changed a minor behavior, and it's similar refactor. And it's mentioned in the last comment: https://github.com/apache/flink/pull/24003#issuecomment-1903758592 3. The second commit, it's `[FLINK-33565][Exception] Restart strategy checks whether current failure is a new attempt`. - The concurrent exceptions depend on the result of `RestartBackoffTimeStrategy`. - Other features might depend on it as well in the future, so I prefer it's a independent-change 4. The third commit is core change of this feature. 5. The fourth commit is that fixing the `numberOfRestarts` metic, it may be a ndependent-change as well. That's why I split them into 4 commits. As I said before, squashing them into one commit is fine for me. Looking forward to your feedback, thanks~ [1] https://flink.apache.org/how-to-contribute/code-style-and-quality-pull-requests/#3-separate-refactoring-cleanup-and-independent-changes -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
