Franck Jullien wrote:
> I'm new to OpenOCD development. I'm currently adding support of
> the openrisc CPU to OpenOCD.

Am looking forward to the new support.


> You are talking about submitting small patch in order to reduce the
> reviewing process. Fine, you're right. However, in gerrit, there is
> very small patches waiting for almost a year.....

Do you count time since first push, or time since last activity?


> As I'm working on openrisc (which is almost ready to submit now),
> I know it won't be upstream (or event reviewed) before a very long
> time and it doesn't encourage me to continue on some other
> OpenOCD development.

What Spencer said.


> Could we have a "next" branch where patch could be applied (after
> review of course) then, after some time and test, those patch could
> be cherry-picked to master ?

To me you have just described gerrit. It works *exactly* like that.

Except one thing - everything in gerrit can be used *immediately*
after it has been pushed, there is no review needed for others to
grab changes from gerrit.

If I can help with some instructions for how to grab something out of
gerrit please let me know. It's very easy, and actually it's even
shown on the web page, for every single patch set.

I hope that people understand that gerrit is not some one-way thing?
It is a review tool which expects input from and gives output to
everyone in the project. Or does everyone think that it is for
push-commit-and-wait-until-a-maintainer-includes-the-commit-in-openocd.git
only? You would be sorely mistaken. Please think about wtf the tool
allows you to do and USE it.


> I know it is a lot of work to review patch and it's a lot of
> responsibility to apply patches to the OpenOCD master branch
> as we don't want to break something.

Please spend some time reviewing commits. You might find that commits
with some trivial issues are much more common than commits with
actual errors in logic, and while trivial issues are also important
and deserve review attention it is quite numbening that they are so
pervasive. I will complain about the smallest things that makes your
commit somehow lower quality than it needs to be and you will
generally think that I am an obnoxious fuck, but I guess that we are
all here because we want to improve OpenOCD, so I do it anyway. But
yes, it takes time to point out issues and try to educate the author
so that they will never produce a commit with the same issue again.
It's really boring to see the same issues over and over.

In contrast, perfect patches require zero time (no issues and no
educating the author needed), they are also quick to review,
and they are a pleasure to include in openocd.git.

Linus wants perfect patches for Linux. Why should we settle for
mediocre in OpenOCD? Because most of the programmers are mediocre?
That's a lame ass excuse for complacency. Try to learn from other
great software and improve yourself, if you notice that you are
lacking.

But here's a hint: There is zero difference between good programming
on a PC and on a microcontroller.

I think the real problem with the OpenOCD community is that noone
really cares about the code. OpenOCD is just a means to an end,
and debugging is an unwanted activity to begin with.


> However, error is human, if a patch breaks something then we can fix it.

The point of doing review at all, as I see it, is to avoid that
OpenOCD ever breaks at all, while also improving the code by avoiding
any issues before they make it into openocd.git.

I guess this is foreign for many. It requires a different mindset
than constantly committing without consideration.

But the reward is significant. It means that openocd.git can be
released at any time, ie. there is no release engineering process.
I have no idea what the purpose of introducing a release engineering
process *after* having gerrit was all about. It makes no sense to me.
Really, we need to just release what is there. If something is not
ready it gets to wait. Too bad.

I would be fine with setting up a cronjob to generate OpenOCD
releases every four months or so using ubuntu style versioning.

That would make the purpose of review efforts more visible, in a
tangible way. Perhaps that would create some motivation.

I don't know why the F* everyone on this mailing list isn't doing
critical review in gerrit? Is it too much work to look at some lines
of code? Really!?

Or maybe it is because you know nothing about any part of OpenOCD
code, and you would have to study the source files that are being
changed for a very long time in order to reverse engineer what the
F* that commit is doing?

Hint: WRITE THAT IN THE REVIEW AND GIVE -2 because it is impossible
for you to understand the commit message. That is perfect review!

But of course you need to be able to tolerate that the author who is
eager to get their work included without having considered anything
about the quality of their work will hate you for being an asshole
complaining about their work because you think it is not good enough.

But regardless of how people react when they are criticized and
pushed to do even better, it *is* the responsibility of the commit
author to write a perfect commit message, which allows YOU to
understand wtf the commit is doing. When they fail to do that, they
*deserve* to be shamed, because they aren't helping make openocd.git
better.

As an author, it does not take very long to reflect on what a commit
does, and write a few lines about the rationale and effect of the
change in a way that people can understand. It takes maybe a few
minutes. Those minutes saves hours of time for the reviewers.

Unless the change is very well understood by the author they actually
have no business proposing it in the first place. Of course authors
fail to see the big picture sometimes. That's the logic errors that
Franck mentioned. That is why we need to write really good commit
messages - to educate our peers on the OpenOCD codebase, so that it
becomes easier for us to educate the next peer on the codebase, so
that fewer of those mistakes happen already in the first commit
pushed.

This is not rocket science!! I think we can do better than having
two or three persons doing review.

Do NOT come up with some lame excuse that you are unmotivated to do
review because you do not have privilege to include commits into
openocd.git.

As you have noticed, the people who do have that privilege are busy.
But they know that they are busy, and if there is a lot of review by
people without inclusion privilege then that will attract attention
to the commit also from those with the privilege - because obviously
there are people interested in that change, and by now many have
already done critical review, maybe there have been a few iterations
of the change, and finally noone can find a single issue anymore, and
everyone considers the commit so perfect that they would be honored
to have been the author.

Remember that those commits require *zero* time to be included
into openocd.git, and are a pleasure to include, even with a busy
schedule.


//Peter

------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812
_______________________________________________
OpenOCD-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openocd-devel

Reply via email to