[Bug 1323334] Review Request: qtpass - Multi-platform GUI for pass

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

 



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



--- Comment #2 from Dave Olsthoorn <dave.olsthoorn@xxxxxxxxx> ---
(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/

-- 
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
http://lists.fedoraproject.org/admin/lists/package-review@xxxxxxxxxxxxxxxxxxxxxxx




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