Bug 481732 - Review Request: stardict-dic-cs_CZ - czech dictionary for stardict
Review Request: stardict-dic-cs_CZ - czech dictionary for stardict
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-27 07:53 EST by Petr Sklenar
Modified: 2009-04-14 14:58 EDT (History)
3 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Petr Sklenar 2009-01-27 07:53:29 EST
Spec URL: http://people.redhat.com/psklenar/stardict-english-czech.spec
SRPM URL: http://people.redhat.com/psklenar/stardict-english-czech-1.0.0-1.fc10.src.rpmCzech dictionaries for StarDict.

Description:

This is autogenerated dictionary for StarDict.
Data were downloaded from following website:
GNU/FDL: http://slovnik.zcu.cz/

Dictionary was generated using:
GNU/FDL Anglicko-Český slovník to stardict converter version 1.2
You can get conversion script from:
http://cihar.com/software/slovnik
Comment 1 Petr Sklenar 2009-01-27 08:06:16 EST
Hello, this is my second package, I am seeking sponsor like at bug 481272

There is GNU/FDL licence in separate file /usr/share/doc/stardict-english-czech-20081201/README

Should I rename it to 'LICENSE' or is it OK? Original author has this name of file for licence.

I made mistake for URL in first comment, this is right URL: 
Spec URL: http://people.redhat.com/psklenar/stardict-english-czech.spec
SRPM URL:
http://people.redhat.com/psklenar/stardict-english-czech-1.0.0-1.fc10.src.rpm

Czech dictionaries for StarDict.
Comment 2 manuel wolfshant 2009-01-27 08:13:36 EST
No, it is not necessary to rename the files from the source (unless there is a very good reason, for instance in order to avoid a conflict with another package)
Comment 3 Lubomir Rintel 2009-01-28 08:29:02 EST
Taking this.

(In reply to comment #1)
> Hello, this is my second package, I am seeking sponsor like at bug 481272

I could sponsor you, but I can't find any reviews you've participated in. Would you mind pointing me to some, so I can be sure about your packaging abilities? In case you haven't done so, I recommend picking up a couple of review requests and doing preliminary reviews. Thanks!
Comment 4 Petr Sklenar 2009-01-28 08:44:51 EST
I started with fedora packaging these time so I haven't done any pre-review, but I would like.

Please can you recommend me some review to do preliminary review.
Comment 5 manuel wolfshant 2009-01-28 08:51:05 EST
Petr, you could start for instance with https://bugzilla.redhat.com/show_bug.cgi?id=480860 which is an easy one.
Comment 6 Petr Sklenar 2009-01-28 09:01:48 EST
re Comment #5:
OK, thanks much
Comment 7 Lubomir Rintel 2009-01-28 09:11:01 EST
Here's the review queue:

http://fedoraproject.org/PackageReviewStatus/REVIEW.html

You are probably looking for bugs in NEW state, assigned to (Nobody).
By skimming through the list I've managed to find one package for which the review did not start at all yet: python-argparse (bug #462505)

You may want to help your colleagues with Merge reviews as well ;)
Comment 8 Petr Sklenar 2009-01-29 09:56:18 EST
Hi,
I made informal review at https://bugzilla.redhat.com/show_bug.cgi?id=480860#c4
Comment 9 Lubomir Rintel 2009-01-29 12:58:28 EST
(In reply to comment #8)
> Hi,
> I made informal review at https://bugzilla.redhat.com/show_bug.cgi?id=480860#c4

I like it. I'd still like to se at least a couple more, this one was pretty easy :)

A review of this package:

1.) Package name.

To be consistent with other existing practice applied to other stardict dictionary packages I suggest you rename the package to "stardict-dic-cs" or "stardict-dic-cs_CZ".

You may still want to add a Provides: for the upstream name.

2.) Package version.

%define dicdate 20081201
Version: 1.0.0
Source0: ftp://dl.cihar.com/slovnik/stable/stardict-english-czech-%{dicdate}.tar.gz

Is 1.0.0 made up? Why not use 20081201 as version?

3.) URL tag.

URL: ftp://dl.cihar.com/slovnik/stable/stardict-english-czech-%{dicdate}.tar.gz

Purpose of the URL tag is to address the package's home page, with general information about it -- it definitely should not be the same as Source tag.

4.) BuildArchitectures tag.

This is fairly minor, but: it is commonly spelled "BuildArch:"

5.) Setup

%setup -c -T -n %{name}-%{version}
%setup -q -n %{name}-%{version} -D -T -a 0

This could be written as:

%setup -q -c -n %{name}-%{version}

6.) No need to rename the README file

install -p -m 0644 %{name}-%{dicdate}/README LICENSE

Especially when it does not contain the license text.

7.) Your %description is not informative

Description is not meant to contain information about history of the package or its build process, but rather contents of the package. The latter URL belongs to the Url: tag and rest should be replaced with something like:

"Czech-English and English-Czech translation dictionaries for StarDict, a GUI-based dictionary software."

8.) Group.

I guess Applications/Text would be a far better pick than Applications/System.
Comment 10 Lubomir Rintel 2009-02-09 09:45:04 EST
Petr: ping. Alive? :)
Comment 11 Petr Sklenar 2009-02-09 11:32:26 EST
hi, yes I live :)

I made changes up your comments, and latest version is here:
 
Spec URL: http://people.redhat.com/psklenar/stardict-dic-cs_CZ.spec

SRPM URL: http://people.redhat.com/psklenar/stardict-dic-cs_CZ-
20081201-2.fc10.src.rpm
Comment 12 Lubomir Rintel 2009-02-19 04:01:04 EST
Sorry for the delay. I think it is much better now, thanks. Radek already sponsored you, lifting FE_NEEDSPONSOR.

APPROVED
Comment 13 Petr Sklenar 2009-03-02 07:39:40 EST
New Package CVS Request
=======================
Package Name: stardict-english-czech
Short Description: Czech dictionaries for StarDict.
Owners: psklenar
Branches: F-10 F-11
InitialCC: psklenar
Comment 14 Kevin Fenzi 2009-03-02 19:28:38 EST
Is the package name right here?
The cvs request has 'stardict-english-czech', comment #11 has 'stardict-dic-cs_CZ'.
Comment 15 Petr Sklenar 2009-03-03 07:55:18 EST
yes , you are right, I am sorry, my mistake.

again:

New Package CVS Request
=======================
Package Name: stardict-dic-cs_CZ
Short Description: Czech dictionaries for StarDict.
Owners: psklenar
Branches: F-10 F-11
InitialCC: psklenar
Comment 16 Kevin Fenzi 2009-03-03 15:36:06 EST
cvs done.
Comment 17 Lubomir Rintel 2009-04-14 14:58:42 EDT
I think this was imported and built.

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