Hi Nicolas, Good input and good to have your perspective on this, thank you!
I started a thread last year [1] exactly to advocate for being brave and innovative and to shift our somewhat conservative culture. This is something I definitely hold near to my heart and I'm happy to say that I think our culture _is_ changing as we are witnessing changes at a rate that is probably faster than the past. I think extreme programming does not tell you to tolerate lower quality code. Instead it tells you don't make big upfront designs, prefer simplicity, iterate quickly, refactor your code, apply Test Driven Development, go for continuous integration, make small releases, and so on. In a sense, it tells you make small continuous improvements instead of trying to go the superhero route which usually fails. So I think the problem being discussed in this thread is not agility, but rather quality; and we are trying to come up with solutions to maintain quality. I think being experimental or innovative does not prevent you from maintaining quality. Now since quality might be a subjective term, I'll try to list some characteristics or traits (please feel free to correct me): - The code at a minimum compiles and passes all tests on your computer before committing. - Code is written carefully and slowly. - Continuously refactor your code to make it cleaner and simpler - If you are not comfortable with a piece of code, ask for opinions before committing - Prefer to have new fresh eyes on your code (pairing is essential in extreme programming) - Avoid quick copy-and-paste patterns and instead prefer refactoring for modularity - Quality wins over quantity - Minimize bulk commits (let's change everything everywhere in the code from X to Y). Usually it fails because the context is different from X to Y I think you get the idea right? This is what we're advocating for in this thread. I don't think we want to slow down, quite the opposite. We just want to maintain quality, which I think is even more important for agility as nothing slows you down more than low quality code. [1] https://lists.apache.org/thread.html/a336af8b3b8092aa8d17abc9b6811a2cb0fec5346ac275bc068d2744@%3Cdev.ofbiz.apache.org%3E On Thu, Jun 29, 2017 at 3:14 PM, Nicolas Malin <nicolas.ma...@nereide.fr> wrote: > It's a difficult subject because I'm sure Jacques realized some change with > the clear aims to improve the framework with his knowledge. > When your knowledge is completed by other commiter's remark you are on the > crossway : > * revert all your work to try to continue to sharing on the resolution to > regenerate a better solution > * continue to improve an fix introduce bug with your mind evolution > The first way is reasonable but with the risk to freeze your initial > motivation if the community aren't react > The second generate more noise and disturb but escape the freeze risk. > Finally I can't have a opinion, each have advantage and inconvenient, > currently I experiment the extreme programming concept and I'm more disturb > now ! > So too much commit or more review, I feel that isn't the real problem > particularity on trunk who it's acceptable to have some break days (warn: > I'm not in favor) but more on the real aims to achieve, the reason to do > that and sharing/understand why we are ready to waste some time on it. > > Cheers, > Nicolas > > > Le 29/06/2017 à 10:43, Jacopo Cappellato a écrit : >> >> +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 >>> >
