[LAU] Ardour: exporting woes

[Gene Heskett]

On Wednesday 30 March 2016 15:31:27 Fons Adriaensen wrote:

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

Phew!  I'm with you 100% Fons. Stay upwind, way upwind of code like this.

For starters, where the heck are the closing } for the two else clauses 
above?

Cheers, Gene Heskett
-- 
"There are four boxes to be used in defense of liberty:
 soap, ballot, jury, and ammo. Please use in that order."
-Ed Howdershelt (Author)
Genes Web page <http://geneslinuxbox.net:6309/gene>