https://bugzilla.redhat.com/show_bug.cgi?id=1323334
--- Comment #2 from Dave Olsthoorn dave.olsthoorn@gmail.com --- (In reply to Antti Järvinen from comment #1)
Hello Dave
Hi, and sorry for the late response, I have been busy the last while.
and thanks for submitting your package for review. I'm not in packagers group so I can't submit your package forward but I hope my comments are useful for someone who can. I have checked some parts of the package and I'm listing issues found below. There is nothing catastrophic but a few items need to be changed. Worst thing actually is that the program has been sitting in "generate gnupg keypair" dialog for half an hour now - is the software functional in F25?
Weird... I don't seem to have this error. Maybe it has something to do with my require on password-store instead of pass (which is the package name).
But, here is list of packaging related issues that I have noticed:
- In fedora jargon GPL-3.0 is called GPLv3.
Fixed in newer version
- Package does not own all directories it creates so in %files section it might be necessary to add lines
%dir %{_datadir}/icons/ %dir %{_datadir}/icons/hicolor %dir %{_datadir}/icons/hicolor/scalable
I feel weird including these in my package.... since hicolor-icon-theme already owns them. I included it as one of the build-requires in the new version
- Instead of "make %{?_smp_mflag}" you should say just
%make_build .. see below for more comments, getting RPM_OPT_FLAGS working deals for example with possible hardening flags that might be very useful with software of this nature.
Fixed. Although that gets added on %configure (%qmake_qt5 here) IIRC.
- Package requires "password-store" but there is no such package in F25. There is "pass" so maybe this is what was meant? With that dependency package never cleanly installs.
Fixed.
- Is it possible to pack the latest version available in github, is there any functional improvements that affect fedora linux setup?
Fixed.
- A minimal manpage would be nice.
I'll discuss with upstream, although this is a GUI application so I don't feel the need is very high.
- You'll need
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache due to icons.
Fixed.
Issues:
- Package installs properly. Note: Installation errors (see attachment) See: https://fedoraproject.org/wiki/Packaging:Guidelines
Fixed, the require on password-store instead of passs was the issue
- gtk-update-icon-cache is invoked in %postun and %posttrans if package contains icons. Note: icons in qtpass See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
Fixed.
[!]: Requires correct, justified where necessary. See comment above about "password-store" vs. "pass".
Fixed.
===== SHOULD items =====
Generic: [!]: Uses parallel make %{?_smp_mflags} macro.
[!]: Package functions as described. Actually no. "pass" is packaged for fedora and installs just fine. Spec file says that qtpass depends on "password-store" and I can't find a package providing that. Changing the requirement line in .spec to "Requires: pass" allows the package to be installed. In F25 chroot in starts, asks for email+pgp password but then keeps on showing "Generate GnuPG keypair" doughnut for 15 minutes .. on 2.4GHz box this seems to be longish time?
[!]: Latest version is packaged. -> Almost latest 1.1.1 seems to be available in github.
Fixed, updated.
[-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
Hmm, I didn't hear of this one before
[-]: %check is present and all tests pass. %check is not present.
No unit tests available
===== EXTRA items =====
Generic: [!]: Rpmlint is run on all installed packages. Note: Mock build failed See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL)
Fixed.
qtpass.src: W: spelling-error Summary(en_US) Multi -> Mulch, Mufti qtpass.src: W: spelling-error %description -l en_US multi -> mulch, mufti qtpass.src: W: spelling-error %description -l en_US unix -> UNIX, Unix, uni
Fixed.
==New URL's== Spec URL: https://daveo.fedorapeople.org/qtpass-1.1.1-1/qtpass.spec SRPM URL: https://daveo.fedorapeople.org/qtpass-1.1.1-1/qtpass-1.1.1-1.fc23.src.rpm
[1]. http://spdx.org/licenses/