[Bug 1361659] Re-Review Request: vdsm - Virtual Desktop Server Manager

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1361659

Yaniv Bronhaim <ybronhei@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(dougsland@redhat.
                   |                            |com)



--- Comment #2 from Yaniv Bronhaim <ybronhei@xxxxxxxxxx> ---
New: Using master branch, so the output tar and src.rpm version is different
than the one specified in the description

See my comments inline below

Spec URL: bronhaim.fedorapeople.org/vdsm.spec
SRPM URL: vdsm-4.18.999-338.gite4625d3.fc23.src.rpm


(In reply to Douglas Schilling Landgraf from comment #1)
> Hi Yaniv,
> 
> fedora-review initially shared below, could you please review the following?
> 
> - All build dependencies are listed in BuildRequires, except for any that
>   are listed in the exceptions section of Packaging Guidelines.

I believe that all our build requirements are listed accordingly - do you see
something specific that relates to exceptions section? 

>   Note: These BR are not needed: rpm-build

It was added by purpose as it was needed in https://gerrit.ovirt.org/#/c/7955/

> 
> - Uses parallel make %{?_smp_mflags} macro.

For some reason the make fails with -j4 flag :\ still trying to understand why,
so for now I still don't use the macro.

> 
> - Do not use config for tmpfilesdir (%config(noreplace)
> %{_tmpfilesdir}/%{vdsm_name}.conf)
> %{_tmpfilesdir} expands to %{_prefix}/lib/tmpfiles.d which is the location
> that the package's default tmpfile creation scripts should install into.
> %{_tmpfilesdir}/%{name}.conf is not marked as a %config file because it is
> not supposed to be edited by administrators. Administrators can override the
> package's %{name}.conf by placing an identically named file in
> /etc/tmpfiles.d/, but this should very rarely be needed. 
> 

Removed %config(noreplace) from this file which is not needed

> The requires of python-devel must be updated
> - Package contains BR: python2-devel or python3-devel

Require specifically python2-devel - we don't need python3-devel for now

> 
> - Try to reduce the number of output of rpmlint

The warnings we have are:

>> vdsm.spec:159: W: unversioned-explicit-obsoletes %{name}-infra

Added >= 4.16.0 where it was first introduced 

>>vdsm.spec:768: E: hardcoded-library-path in %{buildroot}/usr/lib/systemd/systemd-vdsmd
vdsm.spec:769: E: hardcoded-library-path in
%{buildroot}/usr/lib/systemd/system-preset/85-vdsmd.preset
vdsm.spec:904: E: hardcoded-library-path in /usr/lib/systemd/systemd-vdsmd
vdsm.spec:905: E: hardcoded-library-path in
/usr/lib/systemd/system-preset/85-vdsmd.preset

Using %{_libdir} now

>>vdsm.spec: W: invalid-url Source0: vdsm-4.18.999.tar.gz

Changed to https://bronhaim.fedorapeople.org/%{vdsm_name}-%{version}.tar.gz and
I'll upload the tar there

Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]