Bug 463744 - Review Request: screenruler - GNOME screen ruler
Summary: Review Request: screenruler - GNOME screen ruler
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Debarshi Ray
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-24 13:56 UTC by Deji Akingunola
Modified: 2008-10-09 12:21 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-10-09 12:21:07 UTC
Type: ---
Embargoed:
debarshir: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Deji Akingunola 2008-09-24 13:56:18 UTC
Spec URL: ftp://czar.eas.yorku.ca/pub/gruler/screenruler.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/gruler/screenruler-0.85-1.fc10.src.rpm
Description: Screenruler is a small GNOME based utility that allows you to measure objects 
on your desktop. It can be used to take both horizontal and vertical measurement
in 6 different metrics: pixels, centimeters, inches, picas, points, and as a
percentage of the ruler’s length.

NOTE: This is not really a new package, it is a rename of the current gruler package (upstream just settled on a new name).

Comment 1 Debarshi Ray 2008-09-26 14:00:18 UTC
I can not locate the SRPM at the given link.

[rishi@freebook SRPMS]$ wget -c "ftp://czar.eas.yorku.ca/pub/gruler/screenruler-0.85-1.fc10.src.rpm"
--2008-09-26 13:59:29--  ftp://czar.eas.yorku.ca/pub/gruler/screenruler-0.85-1.fc10.src.rpm
           => `screenruler-0.85-1.fc10.src.rpm'
Resolving czar.eas.yorku.ca... 130.63.239.17
Connecting to czar.eas.yorku.ca|130.63.239.17|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD /pub/gruler ... done.
==> SIZE screenruler-0.85-1.fc10.src.rpm ... done.
==> PASV ... done.    ==> RETR screenruler-0.85-1.fc10.src.rpm ... 
No such file `screenruler-0.85-1.fc10.src.rpm'.

[rishi@freebook SRPMS]$

Comment 2 Deji Akingunola 2008-09-26 14:06:36 UTC
(In reply to comment #1)
> I can not locate the SRPM at the given link.
> 
Please try again, it is there now.

Comment 3 Debarshi Ray 2008-10-01 22:24:42 UTC
MUST Items:

OK - rpmlint is unclean on RPM and SRPM
    + [rishi@freebook SPECS]$ rpmlint ../SRPMS/screenruler-0.85-1.fc9.src.rpm 
      screenruler.src: E: description-line-too-long on your desktop. It can be used to take both horizontal and vertical measurement
      1 packages and 0 specfiles checked; 1 errors, 0 warnings.
      [rishi@freebook SPECS]$ rpmlint ../RPMS/noarch/screenruler-0.85-1.fc9.noarch.rpm 
      screenruler.noarch: E: description-line-too-long on your desktop. It can be used to take both horizontal and vertical measurement
      1 packages and 0 specfiles checked; 1 errors, 0 warnings.
      [rishi@freebook SPECS]$ 

OK - follows Naming Guidelines
OK - spec file is named as %{name}.spec

xx - package does not meet Packaging Guidelines and Ruby Packaging Guidelines
    + According to https://fedoraproject.org/wiki/Packaging/Guidelines#Summary_and_description
      lines in the description should not be longer than 80 characters.
    + According to https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines
      each Ruby package must have:
      Requires(abi): ruby
      BuildRequires: ruby
      Is this needed in this case?

OK - license meets Licensing Guidelines
OK - License field meets actual license
OK - upstream license file included in %doc
OK - spec file uses American English
OK - spec file is legible
OK - sources match upstream sources
OK - package builds successfully
OK - ExcludeArch not needed
OK - build dependencies correctly listed
OK - no locales
OK - no shared libraries
OK - package is not relocatable
OK - file and directory ownership
OK - no duplicates in %file
OK - file permissions set properly
OK - %clean present

OK - macros used consistently
    + While in some places you have used %{_name}, you have also used
      'screenruler'. Would be nice if you could use either one of them for the
      sake of consistency.

OK - contains code and permissable content
OK - -doc is not needed
OK - contents of %doc does not affect the runtime
OK - no header files
OK - no static libraries
OK - no pkgconfig files
OK - no library files
OK - -devel not needed
OK - no libtool archives

xx - %{name}.desktop file is invalid
    + According to https://fedoraproject.org/wiki/Packaging/Guidelines#desktop
      desktop-file-validate must be run on the .desktop file, and it says:
      [rishi@freebook SPECS]$ desktop-file-validate /usr/share/applications/fedora-screenruler.desktop 
      /usr/share/applications/fedora-screenruler.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
      [rishi@freebook SPECS]$ 
      The key "Encoding" is deprecated on all supported versions of Fedora.
      Please consider removing it.
    + Has upstream been notified of our .desktop file?

OK - does not own files or directories owned by other packages
OK - buildroot correctly prepped
OK - all file names valid UTF-8

SHOULD Items:

OK - upstream provides license text
xx - no translations for description and summary
OK - package builds in mock successfully
OK - package builds on all supported architectures
OK - package functions as expected
OK - scriptlets not needed
OK - subpackages are not needed
OK - no pkgconfig files
OK - no file dependencies

Comment 4 Deji Akingunola 2008-10-02 02:19:46 UTC
Updated spec file and srpm at;
Spec URL: ftp://czar.eas.yorku.ca/pub/gruler/screenruler.spec
SRPM URL: ftp://czar.eas.yorku.ca/pub/gruler/screenruler-0.85-2.fc10.src.rpm

'BuildRequires: ruby' is not needed for this package.

Comment 5 Debarshi Ray 2008-10-05 11:45:12 UTC
The issue of %{name} and 'screenruler' being mixed in the Spec is still there, but I will leave it to your personal taste. :-)

+---------------------------------+
| This package is APPROVED by me. |
+---------------------------------+

https://www.redhat.com/archives/fedora-packaging/2008-October/msg00004.html suggests an alternative way of renaming packages.

Comment 6 Deji Akingunola 2008-10-07 14:19:32 UTC
New Package CVS Request
=======================
Package Name: screenruler
Short Description: GNOME screen ruler
Owners: deji
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes

Comment 7 Kevin Fenzi 2008-10-07 17:34:39 UTC
cvs done.

Comment 8 Deji Akingunola 2008-10-09 12:21:07 UTC
Imported into cvs and built in koji. Thank you Debarshi Ray for doing the review.


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