+1 Review Than Commit should be used more often. I also wish that Jacques, from now on, will to be less resistant to requests to revert his commits: the act of reverting a commit is not a shame on the author nor a punishment but instead it is a mechanism that helps to keep the code clean while some code changes are under scrutiny or discussion; it also helps to but time to discuss (community discussions take time); the commit history will be also improved because, after the community review, the commit will represent a fully reviewed feature rather than having one initial (disputed) commit followed by one or more commits to fix/improve the original one.
Regards, Jacopo On Wed, Jun 21, 2017 at 12:37 PM, Taher Alkhateeb < slidingfilame...@gmail.com> wrote: > Hello Everyone, > > I am starting this thread because of the latest unexpected release thread > [1] due to a major bug introduced by Jacques Le Roux in [2]. > > We had multiple discussion with Jacques, one such discussion [3] was due to > a bad commit in which I made a recommendation to stop doing bulk commits > and focus instead on slowly refactoring code and Jacopo mentioned in the > same thread that before doing bulk try-with-resources to start a new thread > and discuss this issue. > > We faced multiple quality issues from improper commits. One such issue was > with respect to improperly closing streams [4] in which both Jacopo and > myself asked Jacques to revert and get a better understanding of how > streams work. Other discussions occured around committing quickly without > testing and hence crashing the system in [5] and [6]. > > Jacques continues with his stream of commits [7] and we continue to witness > some negative consequences accordingly. I'm not even sure we caught all > problems yet. > > I think avoiding improperly studied, rushed or bulk commits is important > because such commits are: > - Requireing a lot of time from reviewers > - Difficult to review > - Lowering code quality > > It is therefore my recommendation to agree as a community on reducing such > commits and to ask Jacques and other committers to follow the > review-then-commit process for large / complex commits. > > WDYT? > > [1] https://s.apache.org/clpW > [2] https://issues.apache.org/jira/browse/OFBIZ-9410 > [3] https://s.apache.org/8Wq3 > [4] https://s.apache.org/DpvM > [5] https://s.apache.org/IN2U > [6] https://s.apache.org/c8GG > [7] r1798571 r1798566 r1798353 r1797792 r1797791 r1797790 r1797744 r1797743 > r1797742 r1797373 r1797356 r1797222 r1797161 r1797160 r1797159 r1797158 > r1797155 r1797097 r1797079 r1797074 r1789045 r1788065 r1787949 r1761047 > r1761045 r1761023 r1759944 r1759088 r1759082 r1758951 >
