diff mbox series

qemu.rst: audio: reference to Command-Line options

Message ID 20221109115018.16086-1-atanas.bunchev@konsulko.com
State New
Headers show
Series qemu.rst: audio: reference to Command-Line options | expand

Commit Message

Atanas Bunchev Nov. 9, 2022, 11:50 a.m. UTC
The previous statement can cause confusion, because this is the
first time when the `audio` argument is mentioned.

Signed-off-by: Atanas Bunchev <atanas.bunchev@konsulko.com>
---
 documentation/dev-manual/qemu.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Quentin Schulz Nov. 9, 2022, 12:12 p.m. UTC | #1
Hi Atanas,

On 11/9/22 12:50, Atanas Bunchev wrote:
> The previous statement can cause confusion, because this is the
> first time when the `audio` argument is mentioned.
> 
> Signed-off-by: Atanas Bunchev <atanas.bunchev@konsulko.com>
> ---
>   documentation/dev-manual/qemu.rst | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/documentation/dev-manual/qemu.rst b/documentation/dev-manual/qemu.rst
> index 5a4a82ce6..4fcfd0686 100644
> --- a/documentation/dev-manual/qemu.rst
> +++ b/documentation/dev-manual/qemu.rst
> @@ -125,8 +125,9 @@ available. Follow these general steps to run QEMU:
>      -  This example specifies to boot an :term:`Initramfs` image and to
>         enable audio in QEMU. For this case, ``runqemu`` sets the internal
>         variable ``FSTYPE`` to ``cpio.gz``. Also, for audio to be enabled,
> -      an appropriate driver must be installed (see the previous
> -      description for the ``audio`` option for more information).
> +      an appropriate driver must be installed (see the description for
> +      the ``audio`` option in the Command-Line Options section below
> +      for more information).

We can use a reference here so this always is up-to-date.

I can suggest the following diff:

diff --git a/documentation/dev-manual/qemu.rst 
b/documentation/dev-manual/qemu.rst
index 5a4a82ce6..8f70eec10 100644
--- a/documentation/dev-manual/qemu.rst
+++ b/documentation/dev-manual/qemu.rst
@@ -125,8 +125,8 @@ available. Follow these general steps to run QEMU:
     -  This example specifies to boot an :term:`Initramfs` image and to
        enable audio in QEMU. For this case, ``runqemu`` sets the internal
        variable ``FSTYPE`` to ``cpio.gz``. Also, for audio to be enabled,
-      an appropriate driver must be installed (see the previous
-      description for the ``audio`` option for more information).
+      an appropriate driver must be installed (see the ``audio`` option
+      in :ref:`dev-manual/qemu:\`\`runqemu\`\` command-line options` 
for more information).
        ::

           $ runqemu qemux86-64 ramfs audio

What do you think?

Cheers,
Quentin
Atanas Bunchev Nov. 9, 2022, 2:04 p.m. UTC | #2
Hi Quentin,

I tried to use reference initially, but I couldn't get it to render 
properly in my github clone.

If it renders as expected in the documentation, i do approve the 
suggested change. Thanks!

Cheers,
Atanas

On 11/9/22 14:12, Quentin Schulz wrote:
> Hi Atanas,
>
> On 11/9/22 12:50, Atanas Bunchev wrote:
>> The previous statement can cause confusion, because this is the
>> first time when the `audio` argument is mentioned.
>>
>> Signed-off-by: Atanas Bunchev <atanas.bunchev@konsulko.com>
>> ---
>>   documentation/dev-manual/qemu.rst | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/documentation/dev-manual/qemu.rst 
>> b/documentation/dev-manual/qemu.rst
>> index 5a4a82ce6..4fcfd0686 100644
>> --- a/documentation/dev-manual/qemu.rst
>> +++ b/documentation/dev-manual/qemu.rst
>> @@ -125,8 +125,9 @@ available. Follow these general steps to run QEMU:
>>      -  This example specifies to boot an :term:`Initramfs` image and to
>>         enable audio in QEMU. For this case, ``runqemu`` sets the 
>> internal
>>         variable ``FSTYPE`` to ``cpio.gz``. Also, for audio to be 
>> enabled,
>> -      an appropriate driver must be installed (see the previous
>> -      description for the ``audio`` option for more information).
>> +      an appropriate driver must be installed (see the description for
>> +      the ``audio`` option in the Command-Line Options section below
>> +      for more information).
>
> We can use a reference here so this always is up-to-date.
>
> I can suggest the following diff:
>
> diff --git a/documentation/dev-manual/qemu.rst 
> b/documentation/dev-manual/qemu.rst
> index 5a4a82ce6..8f70eec10 100644
> --- a/documentation/dev-manual/qemu.rst
> +++ b/documentation/dev-manual/qemu.rst
> @@ -125,8 +125,8 @@ available. Follow these general steps to run QEMU:
>     -  This example specifies to boot an :term:`Initramfs` image and to
>        enable audio in QEMU. For this case, ``runqemu`` sets the internal
>        variable ``FSTYPE`` to ``cpio.gz``. Also, for audio to be enabled,
> -      an appropriate driver must be installed (see the previous
> -      description for the ``audio`` option for more information).
> +      an appropriate driver must be installed (see the ``audio`` option
> +      in :ref:`dev-manual/qemu:\`\`runqemu\`\` command-line options` 
> for more information).
>        ::
>
>           $ runqemu qemux86-64 ramfs audio
>
> What do you think?
>
> Cheers,
> Quentin
Quentin Schulz Nov. 9, 2022, 4:27 p.m. UTC | #3
Hi Atanas,

On 11/9/22 15:04, Atanas Bunchev wrote:
> Hi Quentin,
> 
> I tried to use reference initially, but I couldn't get it to render 
> properly in my github clone.
> 

Yeah, you need to escape the backticks in references, not really obvious :/

> If it renders as expected in the documentation, i do approve the 
> suggested change. Thanks!
> 

Can you send a new patch please?

Cheers,
Quentin
diff mbox series

Patch

diff --git a/documentation/dev-manual/qemu.rst b/documentation/dev-manual/qemu.rst
index 5a4a82ce6..4fcfd0686 100644
--- a/documentation/dev-manual/qemu.rst
+++ b/documentation/dev-manual/qemu.rst
@@ -125,8 +125,9 @@  available. Follow these general steps to run QEMU:
    -  This example specifies to boot an :term:`Initramfs` image and to
       enable audio in QEMU. For this case, ``runqemu`` sets the internal
       variable ``FSTYPE`` to ``cpio.gz``. Also, for audio to be enabled,
-      an appropriate driver must be installed (see the previous
-      description for the ``audio`` option for more information).
+      an appropriate driver must be installed (see the description for
+      the ``audio`` option in the Command-Line Options section below
+      for more information).
       ::
 
          $ runqemu qemux86-64 ramfs audio