Bug 448250 - Review Request: nautilus-sound-converter - nautilus extension to convert audio files
Summary: Review Request: nautilus-sound-converter - nautilus extension to convert audi...
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-05-25 00:43 UTC by Brian Pepple
Modified: 2008-06-02 23:10 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-06-02 23:10:02 UTC
Type: ---
Embargoed:
debarshir: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Brian Pepple 2008-05-25 00:43:56 UTC
Spec URL: http://bpepple.fedorapeople.org/rpms/nautilus-sound-converter.spec
SRPM URL: http://bpepple.fedorapeople.org/rpms/nautilus-sound-converter-0.5.0-1.fc9.src.rpm
Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=627414

Description: Adds a "Convert Sound File..." menu item to the context menu
of audio files in Nautilus. This opens a dialog where you can decide what audio
format you wish to convert the selected files to.

Note: When converting a Flac file, the tags won't be written due to this gst-plugins-good bug: http://bugzilla.gnome.org/show_bug.cgi?id=413841

Comment 1 Debarshi Ray 2008-05-31 19:51:46 UTC
MUST Items: 

xx - rpmlint is unclean on SRPM
    + [rishi@ginger SRPMS]$ rpmlint nautilus-sound-converter-0.5.0-1.fc9.src.rpm 
      nautilus-sound-converter.src:24: W: unversioned-explicit-obsoletes
nautilus-flac-converter
      nautilus-sound-converter.src: W: mixed-use-of-spaces-and-tabs (spaces:
line 1, tab: line 13)
      [rishi@ginger SRPMS]$ 

xx - does not follow Naming Guidelines
    + According to
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Renaming.2Freplacing_existing_packages
there should be a versioned Obsoletes, which would also take care of one of the
rpmlint warnings.

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

OK - package meets Packaging Guidelines
    + To preserve timestamps you could consider using:
      make install INSTALL="%{__install} -p" DESTDIR=$RPM_BUILD_ROOT
    + Why not include the AUTHORS, ChangeLog and TODO in %doc?

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 in any of the dynamic linker's default paths
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
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 with sonames
OK - -devel is not needed
OK - libtool archives removed in the spec
OK - %{name}.desktop file not needed
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 are not needed
OK - subpackages are not needed
OK - no pkgconfig files
OK - no file dependencies

Comment 2 Brian Pepple 2008-05-31 21:11:57 UTC
Spec URL: http://bpepple.fedorapeople.org/rpms/nautilus-sound-converter.spec
SRPM URL:
http://bpepple.fedorapeople.org/rpms/nautilus-sound-converter-0.5.0-2.fc9.src.rpm
Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=638449

* Sat May 24 2008 Brian Pepple <bpepple> - 0.5.0-2
- Fix obsoletes.


Comment 3 Debarshi Ray 2008-06-01 14:17:25 UTC
+ There remains an issue with mixed use of spaces and tabs:
[rishi@ginger SRPMS]$ rpmlint nautilus-sound-converter-0.5.0-2.fc9.src.rpm 
nautilus-sound-converter.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1,
tab: line 24)
[rishi@ginger SRPMS]$ 

+ Also are these versioned BuildRequires really needed? Even F-7 seems to have
versions newer than these.
BuildRequires:  nautilus-devel >= 2.12.0
BuildRequires:  gstreamer-devel >= 0.10.3

According to https://fedoraproject.org/wiki/Packaging/Guidelines#Requires :
"... if the lowest possible requirement is so old that nobody has a version
older than that installed on any target distribution release, there's no need to
include the version in the dependency at all. In that case we know the available
software is new enough. ... As a rule of thumb, if the version is not required,
don't add it just for fun."

Comment 4 Brian Pepple 2008-06-01 14:50:39 UTC
(In reply to comment #3)
> + There remains an issue with mixed use of spaces and tabs:
> [rishi@ginger SRPMS]$ rpmlint nautilus-sound-converter-0.5.0-2.fc9.src.rpm 
> nautilus-sound-converter.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1,
> tab: line 24)

Is this *really* a blocker?  I could care less about mixing space & tabs.
 
> + Also are these versioned BuildRequires really needed? Even F-7 seems to have
> versions newer than these.
> BuildRequires:  nautilus-devel >= 2.12.0
> BuildRequires:  gstreamer-devel >= 0.10.3

And again I ask is this really a blocker for not approving a package?

Comment 5 Debarshi Ray 2008-06-01 15:27:18 UTC
(In reply to comment #4)

> Is this *really* a blocker?  I could care less about mixing space & tabs.
> [...]
> And again I ask is this really a blocker for not approving a package?

Although none of them are blockers, they are too trivial to be not fixed. :-) In
any case, I leave it to your discretion.

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

Comment 6 Brian Pepple 2008-06-01 17:44:14 UTC
New Package CVS Request
=======================
Package Name: nautilus-sound-converter
Short Description: Nautilus extension to convert audio files
Owners: bpepple
Branches:
InitialCC: 
Cvsextras Commits: yes


Comment 7 Kevin Fenzi 2008-06-02 03:33:08 UTC
cvs done.

Comment 8 Brian Pepple 2008-06-02 23:10:02 UTC
Debarshi, thanks for the review.

built for devel.


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