[GitHub] [spark] HeartSaVioR edited a comment on pull request #30051: [DO-NOT-MERGE][SPARK-XXXXX][CORE][3.0] Port back SPARK-32557 and SPARK-33146 to branch-3.0

[GitBox <gi...@apache.org>]

HeartSaVioR edited a comment on pull request #30051:
URL: https://github.com/apache/spark/pull/30051#issuecomment-712505206


   There're two points here if I understand correctly, so I'll comment each point.
   
   1. Using revert of revert
   
   I never used `revert` to `claim` or `blame` something. That is simply moving the commit out of, nothing else. Same applies to this case, revert of revert. I didn't intend anything, but I'm sorry if you feel I use revert to disagree with your revert. I agree I should cherry-pick the origin commit again instead of doing revert of revert. My bad.
   
   2. Necessity of creating a backport PR
   
   I'm sorry but I disagree, because of the different view on root cause of the issue.
   
   The original commit was wrong because SPARK-32557 didn't land to branch-3.0, despite it was a follow-up of SPARK-32529 which was landed to branch-3.0. While the type of SPARK-32557 is marked as "improvement" (I guess that was the reason it didn't land to branch-3.0), I can simply turn it to "bug" as it actually fixes a bug in real world. 
   
   That's a two sides of the coin - unless the improvement doesn't change the functionality, it is somewhat likely possible to fix a bug. The strict rule about when to backport (backport only for a bug type) is really not pragmatic and that should be only enforced when we don't believe others (while that can be still used as a guideline). Is it the case?
   
   In my view I did two works 1) SPARK-32557 is missing in branch-3.0 so I "corrected" it. 2) After that I cherry picked SPARK-33146 to branch-3.0 again as "usual practice" on merging phase. To not break again I made WIP PR to confirm these works don't break anything. I'm not sure I didn't respect some policy here.
   
   Btw, that was my bad I didn't do some verification after cherry-picking and simply pushing. Though that was my bad, I think it's arguable (and probably meaningful) topic that how many verifications can be done during cherry-picking.
   
   IMHO, enforcing to build "without test" on cherry-picked branch is somewhat pragmatic, as it requires around 15+ mins which may be acceptable. That still breaks the flow, but well, we may account it for "responsibility".
   
   Building with test is completely different story - we'll let our development environment be stuck for 3~4 hours, which doesn't seem to be something we want to enforce to mergers. If that is enforced I expect the negative impact on avoiding to port back while it's ideal to port back. (Again this wouldn't happen if we ported back SPARK-32557.)
   
   Always requesting to contributors to make a backport PRs and require them to hang around for more than 4 hours isn't also the solution - I think it's too hard for volunteers (both mergers and contributors). Accounting all of questions I raised, I guess it costs less we tolerate the possibility of test failures and fix it later.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org