Bug 463744 - Review Request: screenruler - GNOME screen ruler
Review Request: screenruler - GNOME screen ruler
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Debarshi Ray
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-09-24 09:56 EDT by Deji Akingunola
Modified: 2008-10-09 08:21 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2008-10-09 08:21:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
debarshir: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Deji Akingunola 2008-09-24 09:56:18 EDT
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 10:00:18 EDT
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...
Connecting to czar.eas.yorku.ca||: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 10:06:36 EDT
(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 18:24:42 EDT
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


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-01 22:19:46 EDT
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 07:45:12 EDT
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 10:19:32 EDT
New Package CVS Request
Package Name: screenruler
Short Description: GNOME screen ruler
Owners: deji
Branches: F-8 F-9
Cvsextras Commits: yes
Comment 7 Kevin Fenzi 2008-10-07 13:34:39 EDT
cvs done.
Comment 8 Deji Akingunola 2008-10-09 08:21:07 EDT
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.