Bug 494199 - Review Request: drascula-international - Subtitles for Drascula: The Vampire Strikes Back
Review Request: drascula-international - Subtitles for Drascula: The Vampire ...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Felix Kaechele
Fedora Extras Quality Assurance
:
Depends On: 494195
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-05 10:34 EDT by Hans de Goede
Modified: 2009-05-18 05:28 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-05-18 05:28:41 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
felix: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2009-04-05 10:34:14 EDT
Spec URL: http://people.atrpms.net/~hdegoede/drascula-international.spec
SRPM URL: http://people.atrpms.net/~hdegoede/drascula-international-1.0-1.fc11.src.rpm
Description:
Spanish, German, French and Italian subtitles for Drascula: The Vampire
Strikes Back.

The review for the main drascula package is bug 494195
Comment 1 Felix Kaechele 2009-04-05 15:29:09 EDT
I'm currently on vacation in your beautiful country and will have a look into your packages tomorrow or so.
Comment 2 Felix Kaechele 2009-04-11 18:03:18 EDT
Is there any news on integrating this into the main drascula package?
I don't want to make a review that proves to be unnecessary afterwards, ya know :)
Comment 3 Hans de Goede 2009-04-12 10:16:40 EDT
(In reply to comment #2)
> Is there any news on integrating this into the main drascula package?
> I don't want to make a review that proves to be unnecessary afterwards, ya know
> :)  

There is no discussion about integrating this into the main drascula package. There only is some discussion about un-integrating (so making separate) the 
music out of the main package.

IOW go ahead and review this, thanks.
Comment 4 Felix Kaechele 2009-04-17 15:07:42 EDT
Here's my review:

The following items have been checked and are ok:

1. rpmlint is silent
2. package name complies to guidelines
3. package meets packaging guidelines
4. sha1sums match:
    [felix@polaris SOURCES]$ sha1sum drascula-int-1.0.zip*
    87d1b63a46bb7f3a2c1a951e8332906ac98e2eec  drascula-int-1.0.zip
    87d1b63a46bb7f3a2c1a951e8332906ac98e2eec  drascula-int-1.0.zip.orig
5. the spec file is beautifully crafted :-)
6. the package builds on all arches (especially well on noarch ;-)
7. Requires and BuildReqs are sane
8. file ownership is ok
9. macro usage is reasonable (although one could argue that the subpackages could be named %{name}-*)
10. package contains content
11. has great .desktop files
12. doesn't own stuff it shouldn't own

The following items need to be addressed:
1. License seems to be called "Revolution Software Freeware License" (http://liberatedgames.org/licenses/Revolution_Software_Freeware_License.txt). Does this affect the naming of the License in the spec? Is this GPLv2+ at all?

When the license question is cleared out I will approve this package.
Comment 5 Hans de Goede 2009-05-15 11:38:10 EDT
(In reply to comment #4)

> The following items need to be addressed:
> 1. License seems to be called "Revolution Software Freeware License"
> (http://liberatedgames.org/licenses/Revolution_Software_Freeware_License.txt).
> Does this affect the naming of the License in the spec? Is this GPLv2+ at all?
> 
> When the license question is cleared out I will approve this package.  

Hmm, I've no idea how I ended up with GPLv2+ in the license tag here, a copy
and paste error probably, sorry.

The correct license tag should be:
License:        Freely redistributable without restriction

The license has been checked (for another Revolution_Software_Freeware_License
game) and been ok-ed by Spot:
http://www.redhat.com/archives/fedora-extras-list/2006-November/msg00030.html

Here is an updated version:
Spec URL: http://people.atrpms.net/~hdegoede/drascula-international.spec
SRPM URL:
http://people.atrpms.net/~hdegoede/drascula-international-1.0-2.fc11.src.rpm


p.s.

1) Can you also review drascula-music please, after some discussion it was
   decided it is better to have the music in a separate package instead of
   having it in the main package, see bug 494197

2) How are things going with the openttd open content package ?
Comment 6 Felix Kaechele 2009-05-16 13:21:25 EDT
Okay. That seems to fit.

This package is APPROVED!

I will of course also review your drascula-music package.

Regarding the openttd package: grfcodec is now in Fedora. I was preparing for my final exams in school so I was somewhat lacking time. But now I have loads of free time to kill :) That means openttd will be in Fedora pretty soon.
Comment 7 Christoph Wickert 2009-05-16 13:42:03 EDT
Isn't %{_datadir}/drascula/ unowned? The package has no requirements on something that owns %{_datadir}/drascula/ (in fact it has no requirements at all). I guess "Requires: drascula" is missing, right?
Comment 8 Christoph Wickert 2009-05-16 13:49:31 EDT
Sorry for the above comment, was ether a firefox or pebcak error. Searched the spec for "Requires" and FF gave me no results.
Comment 9 Hans de Goede 2009-05-17 04:17:10 EDT
(In reply to comment #6)
> Okay. That seems to fit.
> 
> This package is APPROVED!
> 

Thanks!

New Package CVS Request
=======================
Package Name:      drascula-international
Short Description: Subtitles for Drascula: The Vampire Strikes Back
Owners:            jwrdegoede
Branches:          F-10 F-11
InitialCC:
Comment 10 Kevin Fenzi 2009-05-18 00:18:54 EDT
cvs done. 

(The music was only branched for devel/F-11, did you mean for there to be a F-10 in that request as well?)
Comment 11 Hans de Goede 2009-05-18 05:28:41 EDT
(In reply to comment #9)
> cvs done.  

Thanks! Imported and build, closing.

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