On 22/03/15 19:22, Ian Clarke wrote: > I think you've misidentified the problem and therefore the solution. The > problem is that diffs would ever be too big to understand on their own, > therefore the solution is that the diff should never be too big to > understand on it's own. A branch/pull-request should rarely represent more > than a few days of work, and thus should rarely take more than a few > minutes to review. ... > It should never be necessary for changes to be that extensive. In my > day-job we're working on a far more complex codebase than Freenet, and yet > a branch/pull-request almost *never* represents more than 4 days of work, > and therefore code reviews rarely require more than 10-20 minutes. Unfortunately, sometimes it is necessary to make big changes.
For example, my work last summer: We can't keep on using Db4o because it's been discontinued. We can't easily move to another object database because they are not standardised. In any case we need to rewrite it to a simpler structure for stability and performance reasons. But this meant rewriting a lot of core code, adding necessary infrastructure, and then implementing backward compatibility. Since this will break APIs, we may as well fix some of the other problems at the same time. Now, if I'd had time, I would have broken this up into a series of pull requests (or big commits) to be reviewed separately. However, most of them cannot be merged independantly - either they introduce new functionality that isn't used (so why should it be in the codebase?), or they break backwards compatibility (which is a largish chunk of work itself). It appears that time was a factor in Xor's case too - he was worried about starting his thesis soon. I believe Ian's answer will be that time is always a factor and the correct solution is to cut corners and hope for the best - we will clean up the code some day. Some day never comes, because there's always another deadline (for employees)... IMHO this is a management style which does not work with the kind of code that Freenet is, that modern OSS is, that Google's infrastructure is. It works well enough with code that doesn't need to be maintained, and it just about works if you can rewrite it completely every year, or ship it once and forget it. Re volunteers, if they know what the rules are and if maintainers respond quickly there's a good chance that they will help to solve the problems, and the code will get merged. >> Longterm this needs to a situation where no one can understand the >> whole code well enough to change it efficiently, which in turn leads >> to pseudo-ownership over certain sub-sections of the code. > If we're producing code that isn't comprehensible without external > explanation, then we're not producing good code. Well written code > shouldn't require an explanation, it should be self-explanatory. This book > lays out some excellent guidelines for this kind of code, every programmer > should read it: http://amzn.com/0132350882 Yes, and allowing everyone to commit with no supervision is a perfect recipe for the sort of spaghetti code that we would like to avoid.
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Devl mailing list Devl@freenetproject.org https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl