Nobody is denying that your adding topological sort was a significant (and valuable) piece of work.
Nobody is denying that there were inconsistencies in the coding style beforehand.The project NEEDS your topo sort changes. The project cannot AFFORD the patch as it was presented.
On Thu, Mar 31, 2016 at 3:00 PM, Fons Adriaensen <fons@xxxxxxxxxxxxxx> wrote:
On Thu, Mar 31, 2016 at 12:46:02PM +0200, Robin Gareus wrote:
> On 03/30/2016 11:40 PM, Fons Adriaensen wrote:
> > But I can live with that. But not with indentation that
> > completely obfuscates code structure. I had good reason
> > to modify the layout of the code where I did that, it
> > was a mess to start with.
>
> How is mixing whitespace and actual code-change in a single patch not
> exactly the same kind of of obfuscation and mess? The diff is just as
> unreadable as the source that was criticized for being unreadable.
If you want to review code you shouldn't read a diff - it doesn't
tell you anything at all on how the code is supposed to work.
Read the result of applying the diff. And before uncrustification
that was *very* readable, and much better documented than the
original.
Please keep in mind that this patch was not just some bugfixes
to an existing algorithm. It implements a completely new one,
including some new data structures, and there was no other option.
There is *no way* this could ever be done as a series of small
incremental changes. Which again means that reading diffs is
a completely useless way to verify the correctness of the
new code.
At the same time this part of the code is hooked into a lot
of other stuff - basically everything related to creating or
deleting clients, ports, and connections.
You may have noticed that while the original algo would
scan all connections of all ports of all clients every
time anything changed, relevant or not, the new one takes
a different approach. It has two levels, one that maintains
some data structures that reflect the effective dependencies
between clients, and a second that calculates the running
order whenever these dependencies change *and only then*.
For this to work, I had to be sure that every port creation
or connection would be matched by a disconnection and deletion,
no matter how and why a client would be removed (and as far
as I was able to find out that was the case). That is only
one of the reasons why I had to read and completely understand
some other parts of the code, something made quite a challenge
by a mix of random code layout, misleading and confusing
function names, and a general lack of documentation on how
things were supposed to work.
Anyway, I'm done with this. The version I submitted a patch
for has been working perfectly here and in some other places,
and in some quite challenging conditions involving tens of
clients coming and going and crashing, and hundreds of port
connections. Of course that doesn't prove it's perfect and
free of errors, but if any problem arises I'll try to debug
it using the original code.
Ciao,
(to R.G: the dichotomy arrived, thanks)
--
FA
A world of exhaustive, reliable metadata would be an utopia.
It's also a pipe-dream, founded on self-delusion, nerd hubris
and hysterically inflated market opportunities. (Cory Doctorow)
_______________________________________________
Linux-audio-user mailing list
Linux-audio-user@xxxxxxxxxxxxxxxxxxxx
http://lists.linuxaudio.org/listinfo/linux-audio-user
_______________________________________________ Linux-audio-user mailing list Linux-audio-user@xxxxxxxxxxxxxxxxxxxx http://lists.linuxaudio.org/listinfo/linux-audio-user
