[Bug 1372670] Review Request: driverctl - device driver control utility

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

 



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



--- Comment #2 from Panu Matilainen <pmatilai@xxxxxxxxxx> ---
(In reply to Igor Gnatenko from comment #1)
> > Group:		System Environment/Kernel
> not needed

Not harmful either. I'm old school and prefer to keep it, thanks.

> 
> > License:	LGPLv2
> LGPLv2+ actually

The README states the license is LGPL v2.1 specifically.

> 
> > # "make archive" from git checkout of tag %{version} or
> > # https://gitlab.com/pmatilai/driverctl/repository/archive.tar.gz?ref=%{version}
> > Source0:	%{name}-%{version}.tar.gz
> Source0:
> %{url}/repository/archive.tar.gz?ref=%{version}#/%{name}-%{version}.tar.gz

I dont get the point, if anything that's LESS readable.

> > Requires(post,postun):	systemd
> %{?systemd_requires}

Actually not. The requirement is there for udevadm which at some point lived in
systemd, but in current Fedora its in systemd-udev. I fixed it upstream to be a
file dependency to cover for these differences, and RHEL-7 matters too.

> 
> > make install DESTDIR=%{buildroot}
> %make_install

Shrug, both work. Changed to %make_install upstream since RHEL-7 seems to have
%make_install already afterall.

> > /usr/lib/udev/vfio_name
> Can you file a bug against systemd to add macro %{_udevdir}?
> 
> > %{_sysconfdir}/bash_completion.d/driverctl-bash-completion.sh
> 1. it should go to %{_datadir}/bash-completion/
> 2. directory is not owned

Sigh. %{_sysconfdir}/bash_completion.d/ was owned by filesystem to avoid the
directory ownership issues, and now I'd need to own two extra directories. Is
there an actual requirement documented for this someplace?

> 
> * Description should end with "."
> * install is invocated without "-p", so fix it in upstream as well ;)

Both fixed upstream.

Thanks for the review, I'll post an updated version once we agree on the
nitpicks :)

-- 
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]