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)
_______________________________________________
Linux-audio-user mailing list
Linux-audio-user@xxxxxxxxxxxxxxxxxxxx
http://lists.linuxaudio.org/listinfo/linux-audio-user
