Bug 245878 - Review Request: stardict-dic-en - English(en) dictionaries for StarDict
Review Request: stardict-dic-en - English(en) dictionaries for StarDict
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2007-06-27 01:38 EDT by Hu Zheng
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version: 2.4.2-3.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-07-05 15:22:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
wtogami: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Hu Zheng 2007-06-27 01:38:19 EDT
Spec URL:

Description: English(en) dictionaries for StarDict
Comment 1 Ralf Corsepius 2007-06-27 02:05:29 EDT
Here _datadir seems to be used correctly.

Another remark:
This is pretty clumsy:

if [ ! -d %{name}-%{version} ]; then mkdir %{name}-%{version}; fi
%setup -q -n %{name}-%{version} -D -T -a 0

It can be written much shorter:
%setup -q -c -a 0

[rsp. .... -a 0 -a 1 ... -a N, for packages consisting of more tarballs.]
Comment 2 Hu Zheng 2007-06-27 03:36:34 EDT
How about add another dict next time? So I think it is better to use the same
scheme as other dic spec files.
Comment 3 Ralf Corsepius 2007-06-27 04:15:03 EDT
<In reply to comment #2)
> How about add another dict next time?
Add another -a <N>

> So I think it is better to use the same
> scheme as other dic spec files.
OK, more bluntly: Your construct is a broken hack.

If you prefer one line per packages, then at least do it the rpm way:
%setup -c -T -n %{name}-%{version}
%setup -q -n %{name}-%{version} -D -T -a 0
%setup -q -n %{name}-%{version} -D -T -a 1

Compare what happens using this construct:

+ rm -rf stardict-dic-ja-2.4.2
+ /bin/mkdir -p stardict-dic-ja-2.4.2
+ cd stardict-dic-ja-2.4.2
++ /usr/bin/id -u
+ '[' 1005 = 0 ']'
++ /usr/bin/id -u
+ '[' 1005 = 0 ']'
+ /bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ cd /home/user/src/rpms/BUILD
+ cd stardict-dic-ja-2.4.2
+ /usr/bin/bzip2 -dc /home/user/src/rpms/SOURCES/stardict-edict-2.4.2.tar.bz2

Against what happens with your contruct:
+ '[' '!' -d stardict-dic-ja-2.4.2 ']'
+ cd /home/user/src/rpms/BUILD
+ cd stardict-dic-ja-2.4.2
+ /usr/bin/bzip2 -dc /home/user/src/rpms/SOURCES/stardict-edict-2.4.2.tar.bz2
+ tar -xf -

Comment 4 Hu Zheng 2007-06-27 04:31:48 EDT
Fixed as the same file :)
Thank you!
Comment 5 Kevin Fenzi 2007-06-27 12:30:37 EDT
Note that convention is to increase the release and add a changelog for each
change (even during review). This allows reviewers to tell when an item was
fixed and to confirm that they have the latest version for review. 
Comment 6 Hu Zheng 2007-06-27 21:23:48 EDT
OK, I will do this next time.

Can we approve them now? In fact, these packages come from a approved spec file
ago, so I think they can be stable now.
Comment 7 Parag AN(पराग) 2007-07-01 23:34:49 EDT
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
f164dcb24b1084e1cfa2b1cb63d590e6  stardict-dictd_www.dict.org_wn-2.4.2.tar.bz2
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
- License text is NOT included in package.
- no %doc files as I think dictd_www.dict.org_wn.ifo is not %doc
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains content.
+ no static libraries.
+ no .pc files are present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Not a GUI app.
+ Requires: stardict >= 2.4.2

Comment 8 Hu Zheng 2007-07-02 00:00:54 EDT
Notice, because of bandwidth issue, I didn't upload the newest SRPM, you can
download the old srpm, replace the spec file with the newest one and rebuild to
get the newest srpm file.
Comment 9 Hu Zheng 2007-07-02 01:30:40 EDT
New Package CVS Request
Package Name: stardict-dic-en
Short Description: English(en) dictionaries for StarDict
Owners: zhu@redhat.com
Branches: F-7
Comment 10 Fedora Update System 2007-07-05 15:21:55 EDT
stardict-dic-en-2.4.2-3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

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