Re: [PATCH 2/2] ALSA: emu10k1: use high-level I/O in set_filterQ()

[Takashi Iwai <tiwai@suse.de>]

On Sun, 23 Apr 2023 10:51:38 +0200,
Oswald Buddenhagen wrote:
> 
> On Sun, Apr 23, 2023 at 09:25:39AM +0200, Takashi Iwai wrote:
> > Again, you must have a bit more say here...
> > For example, you didn't write why this change is needed.
> > You thought it obvious?  No, readers don't know.
> > 
> it is obvious from the patch - the code becomes much shorter and more
> legible.

It's not obvious unless you read the code changes.  Not obvious
whether it's a code refactoring without any functional change, etc.
Such info can be well put in the patch description.

> and someone who just reads the log/blame wouldn't care,
> because it doesn't actually explain anything. but whatever.

Someone already cared.  See?

> On Sun, Apr 23, 2023 at 09:35:46AM +0200, Takashi Iwai wrote:
> > On Sat, 22 Apr 2023 18:10:20 +0200,
> > Oswald Buddenhagen wrote:
> >> 
> >> ... and also use more pre-defined constants on the way (some of which
> >> required adjustment).
> >> 
> >> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> > 
> > Applied this one, but skipped the patch 2.
> > 
> which is funny, because that commit message misses the obvious "why"
> part as well - it just mentions an additional thing that is unique to
> this patch.
> 
> so to be consistent, you should reject both patches and wait for an
> update.

Right, it was enough for me to reply the same thing again, so I wanted
just to reduce the pile of XXXX.  I'd reject all at the next time :)

> > BTW, it would be really better if we define some macro for the
> > highlevel I/O definition.  It's cumbersome to decode and check
> > manually at review whether the conversion is correct, and it's
> > error-prone.
> > 
> yes, i considered that.
> i also considered many more refactorings, and had to hold myself back -
> there are enough nice-to-have patches in this series as-is.
> i mean, 15 years ago it would have made sense to go crazy, but now the
> hardware is a bit too obsolete to go much beyond what i actually need
> for my project. i'm assuming some people outside the western sphere
> are still using our scrap with linux, but we rarely hear from them, so
> it's hard to know ...

Yeah, that's a dilemma of maintaining the old legacy stuff.


Takashi