From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
Subject: Re: [PATCH] ALSA: pcm: fix snd_pcm_playback_silence() with free-running mode
Date: Thu, 4 May 2023 17:36:11 +0200 [thread overview]
Message-ID: <ZFPQ68qLr2fy1qRs@ugly> (raw)
In-Reply-To: <877ctokv6x.wl-tiwai@suse.de>
On Thu, May 04, 2023 at 04:49:58PM +0200, Takashi Iwai wrote:
>Sorry, that doesn't work. The review is done upon the patch, and if
>this patch can't be reviewed easily, it's simply no-go.
>
that's a self-imposed limitation.
it's beyond me why in 2023 anyone working on a bigger project is still
using a patch-based review process, given the existence of proper review
tools like gerrit (and crutches like github and gitlab).
i always view patches with 10 lines of context, and regularly use the
"expand context" widgets to get even more into view.
in the small projects i maintain i apply more complex patches first and
view them with -U10 or more.
"i don't see enough to judge this" isn't a complaint that would ever
occur to me leveling at a contributor.
>Again, the problem is the mixture; it partially reverts to the
>original code while it modifies some part in some other way.
>
the baseline is irrelevant - it was already broken, and almost
impossible to reason about.
>By reverting the whole and reapplying fixes -- although it'll need
>more steps -- we can see more clearly what change fixes which part.
>
that's not what actually happens.
this is all deeply intertwined code. splitting up the patch will merely
give you the *illusion* of better understanding the pieces. but to
actually make sense of it, you need to see the whole, in its end state,
because there are no fully functional intermediate states.
the original patch was three patches at first. when i was attempting to
write proper commit messages explaining what fixes what, i found that
it's just impossible to untangle it without lying by omission. so i
squashed them and wrote a cumulative description. and you accepted it.
regards
next prev parent reply other threads:[~2023-05-04 15:37 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-04 13:00 [PATCH] ALSA: pcm: fix snd_pcm_playback_silence() with free-running mode Oswald Buddenhagen
2023-05-04 13:08 ` Jaroslav Kysela
2023-05-04 13:25 ` Jeff Chua
2023-05-04 13:30 ` Oswald Buddenhagen
2023-05-04 13:28 ` Takashi Iwai
2023-05-04 13:33 ` Jaroslav Kysela
2023-05-04 13:54 ` Oswald Buddenhagen
2023-05-04 14:49 ` Takashi Iwai
2023-05-04 15:36 ` Oswald Buddenhagen [this message]
2023-05-04 16:34 ` Takashi Iwai
2023-05-04 17:06 ` Jaroslav Kysela
2023-05-04 17:38 ` Takashi Iwai
2023-05-04 17:53 ` Oswald Buddenhagen
2023-05-05 7:17 ` Oswald Buddenhagen
2023-05-05 7:29 ` Jaroslav Kysela
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZFPQ68qLr2fy1qRs@ugly \
--to=oswald.buddenhagen@gmx.de \
--cc=alsa-devel@alsa-project.org \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.