From: Gerd Hoffmann <kraxel@redhat.com>
To: Akihiko Odaki <akihiko.odaki@gmail.com>
Cc: qemu Developers <qemu-devel@nongnu.org>
Subject: Re: [PATCH 2/2] coreaudio: Handle output device change
Date: Thu, 11 Mar 2021 13:50:38 +0100 [thread overview]
Message-ID: <20210311125038.fdtnnpiskl62nxko@sirius.home.kraxel.org> (raw)
In-Reply-To: <CAMVc7JXtmowah7f8DyW3_hgze6622_zF5dVeAE8mXu4sGMenoQ@mail.gmail.com>
On Wed, Mar 03, 2021 at 10:20:20PM +0900, Akihiko Odaki wrote:
> 2021年3月3日(水) 18:27 Gerd Hoffmann <kraxel@redhat.com>:
> >
> > Hi,
> >
> > > status = coreaudio_get_voice(&core->outputDeviceID);
> > > if (status != kAudioHardwareNoError) {
> > > - coreaudio_logerr2 (status, typ,
> > > - "Could not get default output Device\n");
> > > - return -1;
> > > + coreaudio_playback_logerr (status,
> > > + "Could not get default output Device\n");
> > > + return status;
> >
> > This "pass status code to caller" change can and should be splitted to a
> > separate patch.
> >
> > Likewise with the logging changes.
> >
> > That makes the patch with the actual functional change to deal with the
> > device changes smaller and easier to review.
> >
> > thanks,
> > Gerd
> >
>
> Actually the code was extracted from coreaudio_init_out to
> init_out_device in this patch.
It is likewise good practice to handle such refactorings as separate
patch, i.e. have a patch which reorganizes the code (in this case move
code to the new init_out_device() function) without functional change,
then go add the new handle_voice_change which fixes the actual issue
in the next patch. Finally send the two (or more) changes as patch
series.
take care,
Gerd
next prev parent reply other threads:[~2021-03-11 12:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-01 11:45 [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Akihiko Odaki
2021-03-01 11:45 ` [PATCH 2/2] coreaudio: Handle output device change Akihiko Odaki
2021-03-03 9:27 ` Gerd Hoffmann
2021-03-03 13:20 ` Akihiko Odaki
2021-03-11 12:50 ` Gerd Hoffmann [this message]
2021-03-03 9:22 ` [PATCH 1/2] coreaudio: Drop support for macOS older than 10.6 Gerd Hoffmann
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=20210311125038.fdtnnpiskl62nxko@sirius.home.kraxel.org \
--to=kraxel@redhat.com \
--cc=akihiko.odaki@gmail.com \
--cc=qemu-devel@nongnu.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).