[LAU] Ardour: exporting woes

[Fons Adriaensen]

On Tue, Mar 29, 2016 at 11:12:21AM -0400, Paul Davis wrote:

> (1) I explained to Fons in person and to everyon on the jack-devel mailing
> list what the issues with Fons' original patch. Robin Gareus has eloquently
> restated the basic point, but I'll do it again: the ability to REVIEW code
> changes is critical to long-lived projects, and when a change consists of
> 85% or more white-space/code style changes and only 15% actual functional
> differences, code review becomes all but impossible.

1. All the places where the actual code was changed were
   clearly identified by comments. 

2. When I compare the commit (42393121) that applied my patch
   with previous versions, there are *lots of* major coding
   style and whitespace changes in files that I never touched.
   This means the coding style wasn't consistent to start with.

3. When I read that same commit (which I assume reflects the
   preferred style) I see things like this:

engine.c, line 3884:
-----
        if (dstport->connections && !dstport->shared->has_mixdown) {
                jack_port_type_info_t *port_type =
                        jack_port_type_info (engine, dstport);
                jack_error ("cannot make multiple connections to a port of"
                            " type [%s]", port_type->type_name);
                return -1;
        }

        connection = (jack_connection_internal_t*)
                     malloc (sizeof(jack_connection_internal_t));
        connection->source = srcport;
        connection->destination = dstport;
        connection->srcclient = srcclient;
        connection->dstclient = dstclient;

        src_id = srcport->shared->id;
        dst_id = dstport->shared->id;
        jack_lock_graph (engine);

                if (dstclient->control->type == ClientDriver) {
                        /* Ignore output connections to drivers for purposes
                           of sorting. Drivers are executed first in the sort
                           order anyway, and we don't want to treat graphs
                           such as driver -> client -> driver as containing
                           feedback */
                        VERBOSE (engine,
                                 "connect %s and %s (output)",
                                 srcport->shared->name,
                                 dstport->shared->name);
                } else if (srcclient != dstclient) {
                jack_feedcounts_t *F;
                int dir = 0;

                if ((F = depends_direct (srcclient, dstclient)) != 0) {
                        /* We already have a direct dependency,
                           just increment the connection count. */
                        F->fwd_count++;
                        dir = 1;
                } else if ((F = depends_direct (dstclient, srcclient)) != 0) {
                        /* We already have a reverse direct dependency,
                           just increment the reverse connection count. */
                        F->rev_count++;
                        dir = -1;
----
If this is your idea of indentation that is supposed to help understand
the code structure, then I'm lost. This sort of thing is why I modify
whitespace when working on a file, just to be sure I have the correct
idea of where conditionals and loops start and end. The original code
was full of random indentation like this. 

Ciao,

-- 
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)