This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 507481 - Review Request: moblin-sound-theme - Moblin sound theme
Review Request: moblin-sound-theme - Moblin sound theme
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Christoph Wickert
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FedoraMoblin20
  Show dependency treegraph
 
Reported: 2009-06-22 18:37 EDT by Peter Robinson
Modified: 2009-08-04 07:32 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-08-04 07:32:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
cwickert: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 2 Christoph Wickert 2009-08-03 12:44:47 EDT
Stas tuned for a complete review.
Comment 3 Christoph Wickert 2009-08-03 13:15:58 EDT
OK - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/moblin-sound-theme-0.3-1.fc12.*
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
OK - MUST: named according to the Package Naming Guidelines
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
OK - MUST: Fedora approved license and meets the Licensing Guidelines: CC-BY-SA
OK - MUST: license field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: source matches upstream source by MD5 9f8866cd02f9aa3a0eec659a8ea78e75
OK - MUST: successfully compiles and builds into binary rpms on x86_64 (Note: noarch package)
OK - MUST: all build dependencies are listed in BuildRequires.
N/A - MUST: handles locales properly with %find_lang
OK - MUST: not designed to be relocatable
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf %{buildroot}.
OK - MUST: consistently uses macros
OK - MUST: package contains permissable content
OK - MUST: no large docs
OK - MUST: files included as %doc do not affect the runtime of the application
OK - MUST: package does not contain any .la libtool archives.
OK - MUST: packages does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
OK - SHOULD: builds in mock.
OK - SHOULD: package compiles and builds into binary rpms on all supported architectures.
OK - SHOULD: functions as described.
OK - SHOULD: scriptlets are sane.
N/A - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Other items:
OK - latest stable version
OK - Source0 valid


Issues:
- add NEWS to %doc (README is currently not worth adding)
- preserve timestamps during install (similar to #507480)
- use install rather than cp, e. g.:

install -pm 0644 index.theme %{buildroot}/%{_datadir}/sounds/moblin/
for file in stereo/{*.wav,*.ogg}; do
  install -pm 0644 $file %{buildroot}/%{_datadir}/sounds/moblin/stereo/
done

- Don't add dependencies on /bin/touch: "... ditto if "touch" is not available, there's nothing that would benefit from icon cache updates installed yet either." Source: https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Icon_Cache
If you really think you should require something, use the cureutils package instead of a file based dep.


Fix the issues and consider this package APPROVED.
Comment 4 Peter Robinson 2009-08-03 15:58:12 EDT
Thanks. Will update the points above before committing.

New Package CVS Request
=======================
Package Name: moblin-sound-theme
Short Description: Moblin sound theme
Owners: pbrobinson
Branches: F-11
InitialCC:
Comment 5 Christoph Wickert 2009-08-03 16:03:04 EDT
One more thing: Don't use a disttag here. This is a noarch theme with no dependencies, so it is not necessary to update it during an release upgrade.
Comment 6 Peter Robinson 2009-08-03 16:07:12 EDT
(In reply to comment #5)
> One more thing: Don't use a disttag here. This is a noarch theme with no
> dependencies, so it is not necessary to update it during an release upgrade.  

Actually already fixed in my local copy :-)
Comment 7 Jason Tibbitts 2009-08-03 17:33:56 EDT
CVS done.
Comment 8 Peter Robinson 2009-08-03 18:08:22 EDT
Fixes applied and now built in koji here. Thanks!

http://koji.fedoraproject.org/koji/taskinfo?taskID=1577414
Comment 9 Peter Robinson 2009-08-04 07:32:58 EDT
Built and now in rawhide

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