Bug 722956 - Review Request: relevation - Command-line search for Revelation Password Manager files
Summary: Review Request: relevation - Command-line search for Revelation Password Mana...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Kalev Lember
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-07-18 15:32 UTC by Matthias Saou
Modified: 2012-03-27 21:13 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-03-27 21:13:42 UTC
Type: ---
kalevlember: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Matthias Saou 2011-07-18 15:32:48 UTC
Spec URL: http://thias.fedorapeople.org/review/relevation/relevation.spec
SRPM URL: http://thias.fedorapeople.org/review/relevation/relevation-1.1-1.src.rpm
Description:
Relevation is a tool to retrieve passwords stored in a password file in the
format used by Revelation, from the command-line instead of through a GUI.

Comment 1 Volker Fröhlich 2011-08-24 06:33:25 UTC
Use the name macro instead of "relevation".

If you don't go for EPEL 5 or older, drop the buildroot definition, the clean section and the first rm in the install section.

Defattr is not necessary anymore.

Comment 2 Matthias Saou 2011-08-24 12:58:29 UTC
I clearly intend to build for EPEL5 too, so to keep a common spec file, the buildroot, clean and related will be kept.
Regarding %{name}, I prefer hardcoding the name as it actually makes more sense when some packages are then renamed (like compat-* *<version>).

Comment 3 Till Maas 2011-11-19 16:20:49 UTC
This package name conflicts with the original revelation package:
https://admin.fedoraproject.org/pkgdb/acls/name/revelation
You need to find a different name.

Comment 4 Volker Fröhlich 2012-03-06 23:40:51 UTC
Any news here?

Comment 5 Kalev Lember 2012-03-25 13:18:48 UTC
Taking for review.

(In reply to comment #3)
> This package name conflicts with the original revelation package:
> https://admin.fedoraproject.org/pkgdb/acls/name/revelation
> You need to find a different name.

I don't think there's any naming conflict. This package is:
  relevation
vs the existing one pointed out by Till:
  revelation

Comment 6 Kalev Lember 2012-03-25 13:34:47 UTC
Fedora review relevation-1.1-1.src.rpm 2012-03-25

+ OK
! needs attention

rpmlint output:
$ rpmlint relevation relevation-1.1-1.src.rpm     
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

+ Rpmlint output is clean
+ The package is named according to Fedora packaging guidelines
+ The spec file name matches the base package name.
+ The package meets the Packaging Guidelines
+ The package is licensed with a Fedora approved license and meets the
  Licensing Guidelines.
+ The license field in the spec file matches the actual license
+ The package contains the license file (LICENSE)
+ Spec file is written in American English
+ Spec file is legible
+ Upstream sources match sources in the srpm. md5sum:
  c38d6eb28130bac341ff1547f3f4f477  relevation-1.1.tar.gz
  c38d6eb28130bac341ff1547f3f4f477  Download/relevation-1.1.tar.gz
+ The package builds in koji
n/a ExcludeArch bugs filed
+ BuildRequires look sane
n/a The spec file MUST handle locales properly
n/a ldconfig in %post and %postun
+ Package does not bundle copies of system libraries
n/a Package isn't relocatable
+ Package owns all directories it creates
+ No duplicate files in %files
+ Permissions are properly set
+ Consistent use of macros
+ The package must contain code or permissible content
n/a Large documentation files should go in -doc subpackage
+ Files marked %doc should not affect package
n/a Header files should be in -devel
n/a Static libraries should be in -static
n/a Library files that end in .so must go in a -devel package
n/a -devel must require the fully versioned base
n/a Packages should not contain libtool .la files
n/a Packages containing GUI apps must include %{name}.desktop file
+ Directory ownership sane
+ Filenames are valid UTF-8

Some small nits:
 - The BuildRoot tag, the "rm -rf %{buildroot}" at the beginning of %install section, the whole %clean section, and the "%defattr(-,root,root,-)" line are no longer needed with recent rpmbuild. Feel free to clean this up before importing the package if you want to; it's certainly not blocking the review.
 - Careful when importing the package, because the spec file appears to have fixed email addresses, compared to the source RPM.

Otherwise looks good. APPROVED

Comment 7 Matthias Saou 2012-03-26 08:05:30 UTC
New Package SCM Request
=======================
Package Name: relevation
Short Description: Command-line search for Revelation Password Manager files
Owners: thias
Branches: f16 f17 el5 el6
InitialCC:

Comment 8 Gwyn Ciesla 2012-03-26 12:20:21 UTC
Git done (by process-git-requests).

Reviewer, please take ownership of review BZs, thanks!

Comment 9 Matthias Saou 2012-03-26 12:30:12 UTC
Thanks Kalev for the review, and thanks Jon for the Git setup!

Comment 10 Kalev Lember 2012-03-26 12:41:13 UTC
No problem, happy to help move this along.

By the way, the initial import
http://pkgs.fedoraproject.org/gitweb/?p=relevation.git;a=commitdiff;h=5e230e01a87e26435874b32027317144f8140472
had "Matthias Saou <http://freshrpms.net/>" as the email address in %changelog. Did you mean to replace it with "Matthias Saou <matthias>" as was done in the updated spec file?

Comment 11 Matthias Saou 2012-03-26 12:50:58 UTC
(In reply to comment #10)
> No problem, happy to help move this along.
> 
> By the way, the initial import
> http://pkgs.fedoraproject.org/gitweb/?p=relevation.git;a=commitdiff;h=5e230e01a87e26435874b32027317144f8140472
> had "Matthias Saou <http://freshrpms.net/>" as the email address in %changelog.
> Did you mean to replace it with "Matthias Saou <matthias>" as was done
> in the updated spec file?

Oops, nope. It's the spec vs. srpm difference you pointed out... I didn't think it was going to affect me, as I always import files "manually", but this time I decided to try the "fedpkg import" command using the srpm for the first time.

I'll change that now.


Note You need to log in before you can comment on or make changes to this bug.