Jackie-Jiang commented on PR #9159:
URL: https://github.com/apache/pinot/pull/9159#issuecomment-1205620358

   @siddharthteotia @richardstartin
   To add some context here, we had got multiple PRs merged without following 
the convention, which broke the backward compatibility. It is hard for all the 
contributors and reviewers to remember the convention, especially when the 
change is not next to the comment describing how to keep backward compatible. 
As @FelixGV pointed out, using enum's ordinal is error-prone, and we need to 
use a way to enforce people to follow the convention, and make it easy for 
reviewers to catch when the convention is not followed.
   
   Example PRs that breaks the backward compatibility:
   #9092 Merged then reverted the next day
   #8884 Merged but not caught the backward compatibility issue until this PR 
(already one month!). I actually pointed it out during the 
[review](https://github.com/apache/pinot/pull/8884#discussion_r905529608), but 
the fix goes to the wrong class and I didn't get a chance to review it again. 
This is a proof that we don't have any guard for this convention, except for 
the comment that is 30 lines away from the actual change.


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to