On Mon, Jul 03, 2006 at 12:46:07PM +0200, Lars Gullik Bjønnes wrote: > Abdelrazak Younes <[EMAIL PROTECTED]> writes: > > | Lars, all, > | > | I would like to propose a way forward for new development. Just jump > | to the conclusion if this mail seems too long to read :-) > | [...] > | - The SVN log is here to explain what is in the commits. Reviewers > | that don't know this particular code area just need to verify the C++ > | correctness and to some extent that what is in the patch corresponds > | to what the SVN log says. For reviewers that know this code area, it > | should be very easy to review the _contents_ of the patch, even if it > | does many things at the same time. Of course some arguing could and > | should take place if there is a disagreement. > > What if I onley know one of the areas that the patch touches, and the > other parts is muddled with what I do know and then makes it hard for > me? > What if you happens to know all the areas a patch touches? And it is not an "enormous" patch even though it do more than one thing?
If you really have _trouble_ understanding a patch, feel free to demand that it be broken up. Or, ask someone who knows the other part and say "yes" if both poarts turn out good. However, if you _do_ understand all of it, and reviewing the combined patch is no worse than reviewing two separate patches, please just accept the thing. Lets not turn lyx management into a bureaucrazy where rules must be followed mostly for their own sake. You have good arguments for "one patch - one feature" in general, but please check that they really apply to _this_ particular case before rejecting. > | Let me take my last patch as an example. This patch did two or three > | things but they were all related to the BufferView API cleanup. > > Except the thing that I really bitched about: emit->emitSignal. > That one change make the whole patch unpalatable to me. What I mean exactly. You already like the main part of this patch, the BufferView cleanup. Now you you're rejecting it because of a simple name change? A name change isn't very complicated to understand, even if it is "two" things in one patch. Forcing a new developer to break up a big patch makes sense sometimes, so they learn the way this project works. But it should not be done so often that good developers leave. (A good developer is, of course, someone who have made signigicant contribution. Long time steady work, or big things such as a complete new frontend. :-) There is something called compromise, such as accepting a patch while still pointing out the ways it could be done even better. Accepting a patch that bends the rules now and then makes people think they owe you a favor - perhaps they repay by making a better patch the next time. Punishing people by sticking strictly to rules even when they don't make too much sense doesn't work. That drives people to leave - or in some cases fork the project like what happened to gcc. LyX is GPL too. > | Conclusion: > | > | I propose that the LyX community becomes more of a "meritocracy". Once > | a developer has acquired enough trust from his fellow LyX developers, > | he should be the one who decide what should go in. > > It is not only about trust. Other minds might have other solutons, > perhaps even better ones. > Meritocracy doesn't take away discussions or patch review. It simply means that you won't be the only one able to give a final yes when a solution is implemented. At least consider delegating the power to a few more people who know some part of LyX well. > | I am not saying > | that discussion is forbidden but just that we should not be rigid > | about the "rules". Responsible men do not need rules. > > That sounds almost like a slogan from "NRA" > (And thus I disagree.) Rules are necessary, for there are many ideas about what "responsible" means. But most rules are gross simplifications of reality, and therefore they break when followed too strictly. The rule isn't really "one patch, only one change" (What is a 'change' defined as anyway?) It is more like "a patch shouldn't be too big or complicated", where "one patch, one chage only" is but one example of how that can be achieved. > > Our rules are more like guidelines anyhow (and what film is that quote > from?) > Don't know about films, but Terry Pratchett certainly writes stuff like that. > | So, can I commit my patch? I have some others in the queue... > > is the emitSignal still there? You are really sure that is still worth fighting for? Not because "we have a rule", but because it really makes _this_ patch significantly harder to understand? Helge Hafting
