Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: NetworkManager-openvpn https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=175047 ------- Additional Comments From j.w.r.degoede@xxxxxx 2006-07-26 12:09 EST ------- Okay, here is a full review for this one, I'll do the other 2 you mentioned as time permits. MUST: ===== O rpmlint output is: E: NetworkManager-openvpn explicit-lib-dependency libglade2 E: NetworkManager-openvpn explicit-lib-dependency libgnomeui W: NetworkManager-openvpn no-url-tag W: NetworkManager-openvpn devel-file-in-non-devel-package /usr/lib64/libnm-openvpn-properties.so W: NetworkManager-openvpn non-conffile-in-etc /etc/dbus-1/system.d/nm-openvpn-service.conf E: NetworkManager-openvpn zero-length /usr/share/doc/NetworkManager-openvpn-0.3.2/NEWS W: NetworkManager-openvpn non-conffile-in-etc /etc/NetworkManager/VPN/nm-openvpn-service.name W: NetworkManager-openvpn-debuginfo no-url-tag W: NetworkManager-openvpn no-url-tag W: NetworkManager-openvpn prereq-use %{_bindir}/update-desktop-database W: NetworkManager-openvpn prereq-use %{_bindir}/gtk-update-icon-cache See Must FIX list. * Package and spec file named appropriately * Packaged according to packaging guidelines O License (GPL) ok, but license file not included * spec file is legible and in Am. English. O Source matches upstream ?? (couldn't verify, but you are upstream right?) * Compiles and builds on devel-x86_64 * BR: ok (see below) * No locales * No shared libraries * Not relocatable * Package owns / or requires all dirs (with some strangeness see Must fix below) * No duplicate files & Permissions ok * %clean & macro usage OK * Contains code only * %doc does not affect runtime, and isn't large enough to warrent a sub package * no -devel package needed, no libs / .la files. * no gui -> no .desktop file required MUST fix: ========= * The followjng rpmlint output: E: NetworkManager-openvpn explicit-lib-dependency libglade2 E: NetworkManager-openvpn explicit-lib-dependency libgnomeui Why explicitly require these, explicit requires are ok if you want to force a certain version, but since these requires are versionless, the autodep rpm does on dso's should suffice or? W: NetworkManager-openvpn devel-file-in-non-devel-package /usr/lib64/libnm-openvpn-properties.so Is this needed for dl-opening by NetWorkManager, if not, is this needed at all? W: NetworkManager-openvpn non-conffile-in-etc /etc/dbus-1/system.d/nm-openvpn-service.conf You must mark this %config (just put %config in front of the line) normally we always use %config(noreplace) in FE, bubt I don't know if thats appropiate here. E: NetworkManager-openvpn zero-length /usr/share/doc/NetworkManager-openvpn-0.3.2/NEWS Please don't package this empty file W: NetworkManager-openvpn non-conffile-in-etc /etc/NetworkManager/VPN/nm-openvpn-service.name You must mark this %config (just put %config in front of the line) normally we always use %config(noreplace) in FE, bubt I don't know if thats appropiate here. W: NetworkManager-openvpn prereq-use %{_bindir}/update-desktop-database W: NetworkManager-openvpn prereq-use %{_bindir}/gtk-update-icon-cache Do not use PreReq, it has issues. Instead use Requires(post): xxx and Requires(postun): xxx * Add a Requires(post) (and postun): /sbin/ldconfig * Why no %{?_smp_mflags} after make? either add a comment why tis admitted or add it. * %install must start with: "rm -fr %{buildroot}" * %makeinstall has issues (see f-e-l archives) instead use "make install DESTDIR=%{buildroot}" Should fix: =========== * Do I understand correctly that you are upstream for this package? If so I'll assume that the included tarbal matches upstream. Still it would be nice to provide the tarbal for download from your site and use a full URL as Source0. (rpmlint: W: NetworkManager-openvpn no-url-tag) * You should include a COPYING in the tarbal and in %doc * "rm -f %{buildroot}%{_libdir}/lib*.a" why not just add --disable-static to %configure that reduces build time quite a bit. while you're add it also add --disable-dependency-tracking Questions: ========== * Why is this there? "## %find_lang %{name}" (similar for %files) -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review