Bug 494199 - Review Request: drascula-international - Subtitles for Drascula: The Vampire Strikes Back
Summary: Review Request: drascula-international - Subtitles for Drascula: The Vampire ...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Felix Kaechele
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 494195
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-04-05 14:34 UTC by Hans de Goede
Modified: 2009-05-18 09:28 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-05-18 09:28:41 UTC
Type: ---
Embargoed:
felix: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Hans de Goede 2009-04-05 14:34:14 UTC
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 19:29:09 UTC
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 22:03:18 UTC
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 14:16:40 UTC
(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 19:07:42 UTC
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 15:38:10 UTC
(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 17:21:25 UTC
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 17:42:03 UTC
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 17:49:31 UTC
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 08:17:10 UTC
(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 04:18:54 UTC
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 09:28:41 UTC
(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.