[LAU] Ardour: exporting woes

[Paul Davis]

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.

Nobody is denying that there isn't much in the way of comments to document
how things are supposed to work.

The issue was how to (a) not make things actually worse (by making the
coding style even more inconsistent) and (b) how to improve readability of
the commit, since although it is almost certainly true that reading the
diff alone won't convey the topo sort design, reading the diff *is*
critical is anyone ever needs or wants to understand the *change* that was
made.

A secondary issue has been your continued insistence that submitting a
patch formatted in this way is obviously perfectly OK. I have no idea how
you would contribute to a project with (a) stricter coding standards and
(b) a coding style that you didn't like.

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 at linuxaudio.org>
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 at lists.linuxaudio.org
> http://lists.linuxaudio.org/listinfo/linux-audio-user
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxaudio.org/pipermail/linux-audio-user/attachments/20160331/a40949f6/attachment.html>