Bug 675402

Summary: Review Request: qpass - password manager
Product: [Fedora] Fedora Reporter: Mateusz Piękos <mateuszpiekos>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: bugs.michael, fedora-package-review, martin.gieseking, mrunge, notting, opensource, packages
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard: StalledSubmitter
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-03-02 15:15:38 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Description Mateusz Piękos 2011-02-05 07:25:13 EST
Spec URL: http://qpass.sourceforge.net/qpass.spec
SRPM URL: http://downloads.sourceforge.net/project/qpass/packages/Fedora/qpass-1.0.1-1.fc14.src.rpm
Description: Easy to use password manager which enables you to store your password and logins in encrypted database(using AES-256). Each entry can be named and described.

This is my own application.
This is my first package and I need a sponsor.
Comment 1 Golo Fuchert 2011-02-05 18:02:35 EST
I have a few initial comments on your package:

- there is a typo in the summary (buil-in)
- have you checked if you really need the version numbers in the 
  You need them only, if there are older versions available for currently 
  supported versions of Fedora and maybe RHEL (if you want to package for it).
  Additionally you should check if the Requires are really needed, just from 
  looking at the spec file I would assume not (see http://fedoraproject.org/wiki/PackagingGuidelines#Requires ).
- Lines should not be longer than 80 characters in the description.
- You made a very common mistake in the files section: %{_datadir}/%{name}/*
  This way the package will only own the files _in_ the directory 
  %{_datadir}/%{name}/, if the user now uninstalls the package, the directory 
  will remain on the user's system. So you need to own this by writing 
  %{_datadir}/%{name}/ in the files section. By that, the directory itself _and_
  the files in the directory are owned by the package. http://fedoraproject.org/wiki/Packaging:UnownedDirectories#Unowned_Directories
- Write something in the changelog, even for the very first version of the 
  package. For example
  * Sun Feb 6 2011 Your Name <your_adress@email.com> 0.1-1
  - initial packaging

  If you don't understand the first line, have a closer look at that topic
  in the Guidelines.
- You need to verify the desktop file:
- You need to update the icon cache:

Don't be upset now, the package looks already quite clear for your first try!
Comment 2 Mateusz Piękos 2011-02-06 01:45:57 EST
Thanks for your reply and helpful information. I think that I corrected everything what you said. Here is new:
SPEC: http://qpass.sourceforge.net/qpass.spec
SRPM: http://downloads.sourceforge.net/project/qpass/packages/Fedora/qpass-1.0.1-2.fc14.src.rpm
Comment 3 Martin Gieseking 2011-02-06 15:29:17 EST
Here are a couple of further notes:

- Please drop "Requires: qt, libgcrypt". Both dependencies are detected 

- Don't mix spaces and tabs for indentation. Choose one of them and stick with 

- Replace Group "Application/Internet" with "Applications/Internet"

- Add the missing buildroot dir to the location of the desktop file:
  desktop-file-validate $RPM_BUILD_ROOT%{_datadir}/applications/qpass.desktop

- You should install file COPYING into %{_defaultdocdir}/%{name}-%{version}/.
  One way to do that is to remove the installed file from the buildroot and add
  it with %doc in section %files:
  in %install: rm -f $RPM_BUILD_ROOT%{_datadir}/%{name}/COPYING
  in %files: replace %{_datadir}/%{name}/COPYING with %doc COPYING
Comment 4 Golo Fuchert 2011-02-06 16:10:04 EST
Please include %{_datadir}/%{name}/ in the %files section. Read my post above.
Comment 5 Mateusz Piękos 2011-02-07 09:53:01 EST
Everything should be ok now. I forgot about %{_datadir}/%{name}/ while I was doing changes previously. Here are new:
SPEC: http://qpass.sourceforge.net/qpass.spec
Comment 6 Golo Fuchert 2011-03-01 16:57:23 EST
Looks much better now, just two comments:

As it is stated in the Guidelines, you must not add a file more than once in the %files section. So don't use
the first line includes already the directory itself and everything that is contained in that folder. The second line is neither needed, nor allowed.

Please make sure to update the %changelog and release number _every_ time you change the spec file!

Are you aware of the sponsoring process? You should try to help other people with their packages in order to convince a potential sponsor. For further information see http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group
You sponsor will also make the final review.

Good luck!
Comment 7 Michael Schwendt 2011-03-25 10:34:15 EDT
> Summary:	Easy to use password manager with built-in password generator	

Is the "built-in password generator" really so important as to mention it in the summary? ;-)  And why not advertize it in the %description then?

> %{_datadir}/%{name}/translations/pl_PL.qm

Hmmm, %find_lang --with-qt  cannot be used for this?

Comment 8 Till Maas 2011-11-19 15:10:33 EST
Mateusz, please remove "StalledSubmitter" from the whiteboard when you respond to comment:7.