spec: Explain the BuildRequires on firewalld-filesystem

[libvirt PATCH] spec: Explain the BuildRequires on firewalld-filesystem

Posted by Andrea Bolognani 5 years, 4 months ago

It's not immediately obvious why it is needed.

Suggested-by: Pavel Hrdina <phrdina@redhat.com>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 libvirt.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 6d4eef86ad..c0d84c0e75 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -399,6 +399,7 @@ BuildRequires: rpcgen
 
 BuildRequires: libtirpc-devel
 
+# Needed for the %firewalld_reload macro
 %if %{with_firewalld_zone}
 BuildRequires: firewalld-filesystem
 %endif
-- 
2.26.2

Re: [libvirt PATCH] spec: Explain the BuildRequires on firewalld-filesystem

Posted by Neal Gompa 5 years, 4 months ago

On Thu, Oct 8, 2020 at 7:06 AM Andrea Bolognani <abologna@redhat.com> wrote:
>
> It's not immediately obvious why it is needed.
>
> Suggested-by: Pavel Hrdina <phrdina@redhat.com>
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
>  libvirt.spec.in | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 6d4eef86ad..c0d84c0e75 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -399,6 +399,7 @@ BuildRequires: rpcgen
>
>  BuildRequires: libtirpc-devel
>
> +# Needed for the %firewalld_reload macro
>  %if %{with_firewalld_zone}
>  BuildRequires: firewalld-filesystem
>  %endif
> --
> 2.26.2
>

I'd suggest putting the comment inside the conditional, but otherwise it's fine.

Reviewed-by: Neal Gompa <ngompa13@gmail.com>

-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [libvirt PATCH] spec: Explain the BuildRequires on firewalld-filesystem

Posted by Ján Tomko 5 years, 4 months ago

On a Thursday in 2020, Neal Gompa wrote:
>On Thu, Oct 8, 2020 at 7:06 AM Andrea Bolognani <abologna@redhat.com> wrote:
>>
>> It's not immediately obvious why it is needed.
>>
>> Suggested-by: Pavel Hrdina <phrdina@redhat.com>
>> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>> ---
>>  libvirt.spec.in | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libvirt.spec.in b/libvirt.spec.in
>> index 6d4eef86ad..c0d84c0e75 100644
>> --- a/libvirt.spec.in
>> +++ b/libvirt.spec.in
>> @@ -399,6 +399,7 @@ BuildRequires: rpcgen
>>
>>  BuildRequires: libtirpc-devel
>>
>> +# Needed for the %firewalld_reload macro
>>  %if %{with_firewalld_zone}
>>  BuildRequires: firewalld-filesystem
>>  %endif
>> --
>> 2.26.2
>>
>
>I'd suggest putting the comment inside the conditional, but otherwise it's fine.
>

It looks nicer on the outside. And it matches the style in the rest of
the file.

>Reviewed-by: Neal Gompa <ngompa13@gmail.com>
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano