3 ms·
Most likely, this comment was added in response to a botched attempt to simplify code, to serve as a warning for future maintainers to think twice before making
by omoikane 2y ago
Most likely, this comment was added in response to a botched attempt to simplify code, to serve as a warning for future maintainers to think twice before making a similar attempt.
The commit that added the warning was "Add note about space-shuttle code style"[1], and the one before that was "Revert controller/volume: simplify sync logic in syncUnboundClaim"[2]
[1] https://github.com/kubernetes/kubernetes/commit/de4d193d45f638b8e805a53945e55d6b4b745293 https://github.com/kubernetes/kubernetes/commit/de4d193d45f6...
[2] https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca6c0622c17d6a421ce7b608698801 https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca...
- croshan 2y agoNice find. I think you're right. FWIW, I don't think the code style in [2] is less simple (slightly more readable, to use `if (X) {} else {}` rather than `if (!X) {} else {}`, for example). So to me, this reads as the author of [1] is just overcorrecting by adding process, when some test cases or code review would've been more helpful in preventing whatever incident [2] caused.