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 :-)
|
| Basically, I would like the decision process to be smoothed and based
| on merit. There are some advantages of the one-patch/one-feature
| approach but there also some drawbacks:
|
| The pros:
|
| - For projects with many dozens of developers and a lot of student
| among them, the one-patch/one-feature approach is perhaps the only way
| to proceed as you need to keep the whole in a stable state.
This also happens to be the case with a project with very little
man-power, and where developes have a tendency to come and go.
Half-baked features or features that no-body else than the author
understands is never good.
Sure but this needs to be evaluated on a case by case basis. My patch
was a just cleanup and I reckon that it makes the code easier to
understand. In general, I agree that a feature should be as complete as
possible.
| - With patch oriented review style, it is easier to review a patch
| that does only one thing. We are talking here about deep review, here,
| not only code sanity review.
I look at almost all patches, sure what I most often point at is
trivialities, but my main reason for looking at the patch is to learn
from it and not be alienated from our sourcebase.
Lars, I am maybe saying something obvious but you really don't have the
time to do so. I know I have have the time to look at the patches
related to inset and change-tracking for example. You need to delegate.
| The cons for LyX:
|
| - We are not students (except for Jose which is a bit late in his
| Ph.D. ;-)). I guess most of us have a day job and cannot afford to
| spend too much time on LyX. So when one invests some time, he tries to
| do as much as he can. Wasting time on patch procedure process is
| definitely not fun.
I would expect a student to not get the one-change -> one-feature
rule, but a professional in the business?
I am a scientific who happens to like C++ development (and who think he
is quite good at it ;-)). I am not a professional in the software
business and don't to be. I guess most LyX developers are the same as
me, aren't they?
I develop LyX for fun, putting professional rules to it makes the coding
much less attractive to me.
If you develop your changes
the correct way from the beginning there will be no wasted time on
patch procedures.
I disagree... I develop changes my way and I am free to develop anyway I
like. Your correct way is not _the_ correct way.
But please note: One change might very well be, "Remove GuiWorkarea
and GuiCursor out of BufferView."
(But if you then also add -> "and change emit to emitSignal" then that
is not the case anymore.)
If you really think what you are saying here then you should have said
it before. For me this "emit" problem was an insignificant detail.
| - I estimate that only 2 or 3 developers know at the same time a given
| area of the source code. This doesn't facilitate patch review.
One-change -> one-patch has not much to do with patch review.
So explain what it is all about. And please don't put religious things
like "we should stick to the rule".
(But it helps... instead of making the review process harder (for the
reviewr) it mkaes it easier, and thus speeds it up.)
This, I can understand. But did I say already that I hate patch review
style?
IMHO, apply the patch and read the code, that's my correct way of
reviewing a significant change (bug corrections or incremental change
are not in this case).
| - 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?
Sure it could well be that the patch touches many things at the same
time. In this case, the patch is simply not readable and you really
should read the resulting code instead. In my case, the "emit" thing
touched completely separate files so they were not interfering with the
other changes. So, in my case, this point is void.
This is actually a good example of what I meant: please don't
generalize, don't force one rule for all patches just because this rule
would help for some patches.
| 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.
You are exaggerating and you know it.
| 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.
Sure and they are free and welcome to offer their solution. Actually, I
think I interfered quite a lot in Edwin's and Peter's work and they
accepted or rejected my advice depending on the case. That's what I call
collaborative work.
| 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.)
Far far from it. We are speaking about a fun project not about society.
Of course society needs rules. I am not an anarchist and I am really
against this thing that Americans call their right. Right to kill other
people? Please...
Our rules are more like guidelines anyhow (and what film is that quote
from?)
| So, can I commit my patch? I have some others in the queue...
is the emitSignal still there?
No. As you removed the "emit" macro, this change is not needed anymore.
But I think that "emitSignal" is more meaningful in this case.
Abdel.
