While your points are valid, newdev is his baby and he should be allowed to do with it as he damn well pleases. Don't tell me you've never done this kind of operation locally on your disk? Christopher just chose to commit it. It's why I now run a local branch and only commit final code; it lets me do what I want with code that I wrote 100% of, without being criticzed about design choices before the application is complete.
Note: If this wasn't his app or was a really old app we depend on, I would agree with you. But as far as I'm concerned this is his application and for now there's no point in reviewing it until he considers it done and ready for review.
Best regards, Alex Ionescu
Casper Hornstrup wrote:
~1700 lines to add a check? In the future, please do reformatting in its own patch without semantic changes. This patch would require an hour or more to review if anyone dared to do it and even if someone did, that person would have be superhuman to be able to keep track of that many lines of information.
The risk of introducing a bug is higher with some types of operations than others. Formatting is a relatively safe operation while semantic changes to complex code is a highly risky operation. This means that reviewing a formatting patch is not as important as reviewing a patch with semantic changes to complex code (and thus it doesn't hurt so much if a formatting change is large). When mixing safe and unsafe operations, the overall risk of a introducing a bug in a given patch is the same as if the changes were spread over several patches. However, the risk of the individual patch is equal to risk of the most risky operation in the patch. So if a 1 line highly risky change is hidden in a 1700 line otherwise safe patch, then the overall risk for the whole patch is highly risky.
It may not seem important to have only a few bugs in releases now. We can always use the excuse that ReactOS is alpha software. However, if we ever want to have an 1.x release which is more than a toy-OS, then we have to learn to build better software. And I would prefer that we learn it before 1 day before the deadline for 1.0.0 since that might give us some years to get really good at it. I've written several school papers the night before the deadline, and they were never any good ;-( Another side effect of starting early is that some might even be able to use ReactOS before 1.0.0.
Casper