Hi Jacopo,I'm not for a general change to Review-Then-Commit but pleading for a more sensible way to handle these things as a responsible committer.
As a guideline: if in doubt, always provide a patch and describe what the change should do or fix and let others review.
Of course not for trivial changes or fixes. But when it comes to new modules, significantly changing the behaviour of the business logic etc. I think it's better to apply Review-Then-Commit.
And of course, if others want a commit to be reverted, we should do it first and then discuss.
Regards, Michael Am 27.03.17 um 10:07 schrieb Jacopo Cappellato:
On Sat, Mar 25, 2017 at 1:21 PM, Michael Brohl <[email protected]> wrote:+1 The lack of code documentation is not a free ticket to just change the code behaviour without proper analysis. The right process should be 1. discuss 2. provide a patch 3. let others review/comment 4. decide 5. commit It is really dangerous to easily change code like this. Jacques, please be not so hasty with committing stuff. We have had a lot of similar cases with reverts, committing half done solutions and such lately. And please be aware that others might not have so much time to follow every commit in detail, analyze and comment promptly. It really worries me because we lose quality and it's not easy to detect errors and changed functionality in such a complex project. And don't rely too much on the tests as we don't have such a high test coverage. Thanks for some more patience, MichaelI am fine with continuing with the Commit-Then-Review approach (and leverage Review-ThenCommit approach only when more appropriate) provided that the committer is willing to accept (by reverting or performing further work as requested by the reviewers) negative reviews. What we should really avoid is committers dragging their feet, pushing back reviews, trying to argue and negotiate to keep their code as committed. Jacopo
smime.p7s
Description: S/MIME Cryptographic Signature
