Bug 545405 - Review Request: goldendict - A feature-rich dictionary lookup program
Review Request: goldendict - A feature-rich dictionary lookup program
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks: RussianFedoraRemix
  Show dependency treegraph
 
Reported: 2009-12-08 08:55 EST by Howard Ning
Modified: 2014-10-20 07:47 EDT (History)
8 users (show)

See Also:
Fixed In Version: goldendict-0.9.0-9.20100307git83115ad.fc11
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-04-26 22:17:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mtasaka: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Howard Ning 2009-12-08 08:55:15 EST
This is my first package, I am looking for sponsors. Thank you very much!

Spec URL: 
http://sites.google.com/site/libertysopenworld/Home/goldendict.spec?attredirects=0&d=1
SRPM URL: 
http://sites.google.com/site/libertysopenworld/Home/goldendict-0.9.0-0.1.20091111git.fc12.src.rpm?attredirects=0&d=1
Description: 
Use of WebKit for an accurate articles' representation, complete with all formatting, colors, images and links; Support of multiple dictionary file formats and MediaWiki-based sites; Word suggestion; Full unicode support; Scan popup functionality; Tabbed browsing and more.
Comment 2 Yijun Yuan 2009-12-10 06:44:26 EST
firstly, make sure you have a FAS account, and join the packager group.

secondly, pickup some another package to review, in order to show your understanding to packaging guidelines.

there is no sponsors in FZUG community (yet), however I believe many people from FZUG can help to review your package, and we may redirect some sponsors to you. Just keep active! Or you and your package will be forgotten.

btw your updated spec link is wrong.
Comment 4 Howard Ning 2009-12-10 08:44:07 EST
(In reply to comment #2)
> firstly, make sure you have a FAS account, and join the packager group.
> 
> secondly, pickup some another package to review, in order to show your
> understanding to packaging guidelines.
> 
> there is no sponsors in FZUG community (yet), however I believe many people
> from FZUG can help to review your package, and we may redirect some sponsors to
> you. Just keep active! Or you and your package will be forgotten.
> 
> btw your updated spec link is wrong.  

Thank you very much for providing me such valuable information. I will try to review some other packages and keep active! I am using fedora for about 4 years and really want to contribute something to the fedora.
Comment 5 Yijun Yuan 2009-12-10 09:49:29 EST
I cannot connect to http://site.google.com because of, you know, our Great Fire Wall of China. Please consider to use another service that is not blocked, and is easy to reach (provide a clean URL).
Comment 7 Yijun Yuan 2009-12-10 19:46:47 EST
I can read it now. Please note the bugzilla archive is open to all and open to search, is it OK to show the username and password?

The spec is quite well written in my opinion, congratulations!

Please fix %doc, at least there should be a COPYING which is the GPL3 text. I've not built it on my own system yet, will try it later.

The git clone command is repeated twice.
Comment 8 Howard Ning 2009-12-10 23:35:03 EST
It is a bad idea to show the ftp user and pass. But I will change my password if I can find out a better place to host the files.

I have updated spec file and srpm to include the license file as doc.Also I have changed the group to Application/System according to stardict's spec file. Thank you very much for the advise and encouragement.

SPEC:
ftp://xth_4709239:123456@ftp.xtreemhost.com/htdocs/goldendict.spec 
SRPM
ftp://xth_4709239:123456@ftp.xtreemhost.com/htdocs/goldendict-0.9.0-2.20091209gitc83b6cd.fc12.src.rpm
Comment 10 Pavel Alexeev 2010-01-03 09:06:03 EST
Spec and srpm links is broken.
Comment 11 Howard Ning 2010-01-04 08:17:06 EST
I have tried link, it is working.
Comment 12 Howard Ning 2010-01-04 08:20:04 EST
I have uploaded SRPM here:
http://ifile.it/j3ceih5/goldendict-0.9.0-3.20091226git7a03248.fc12.src.rpm
and spec file:
http://pastebin.com/m29f19a4
Comment 14 Robin Lee 2010-03-07 11:14:14 EST
* Build failure
  - Your srpm does not build on rawhide:
    https://koji.fedoraproject.org/koji/taskinfo?taskID=2036675
    Missing BR: phonon-devel

    This is a successful build with phonon-devel added as BR:
    https://koji.fedoraproject.org/koji/taskinfo?taskID=2036678
Comment 15 Howard Ning 2010-03-07 12:21:36 EST
Thank you very much for reporting the problem. The upstream add the requirement phonon but I forgot to add the requirement in SPEC.

Here is an updated version
SPEC:
http://pastebin.com/L5Pf8GFA
SRPM:
http://ifile.it/an6xvp0/goldendict-0.9.0-5.20100307git83115ad.fc12.src.rpm
Comment 16 Howard Ning 2010-03-07 12:45:56 EST
Why I always fail to build using koji? Thanks.

$ koji build dist-rawhide goldendict-0.9.0-5.20100307git83115ad.fc12.src.rpm
....
2036716 build (dist-rawhide, goldendict-0.9.0-5.20100307git83115ad.fc12.src.rpm): open (ppc10.phx2.fedoraproject.org) -> FAILED: ActionNotAllowed: policy violation
Comment 17 Howard Ning 2010-03-09 10:30:26 EST
Koji build of the new package:
https://koji.fedoraproject.org/koji/taskinfo?taskID=2038695
Comment 18 nucleo 2010-03-15 09:05:43 EDT
(In reply to comment #17)
> Koji build of the new package:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=2038695    

Translations are missing in this build.
Comment 19 Howard Ning 2010-03-15 10:12:00 EDT
necleo, Thank you very much for reporting the problems. I have updated the package and add the missing translations.

SPEC:
http://pastebin.com/5Aqme0Gn

SRPM:
http://ifile.it/elaz57r/goldendict-0.9.0-6.20100307git83115ad.fc12.src.rpm

Koji:

http://koji.fedoraproject.org/koji/taskinfo?taskID=2053625
Comment 20 Mamoru TASAKA 2010-04-09 14:45:39 EDT
Interesting package.

Some notes:
? git based source
  - Would you explain why you have taken the source based from
    git tree instead of the released one?

! BuildRoot
  - BuildRoot is no longer needed (and simply ignored)
    https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* BR
  - ">= 4.5" on "BR: qt-devel" is not needed because all packages
    on currently supported Fedora branches all satisfy this packages.
    ( See the last line of
      https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires )

! Idendation
  - When you divide one command line to several line using backtrash,
    would you add some identation to make it easier to see?

* desktop file
  - Once you install desktop file by desktop-file-install,
    desktop-file-validate is no longer needed.

  - It is better that you use "Icon=goldendict" (i.e. using only file name,
    without extension).
    https://fedoraproject.org/wiki/Packaging/Guidelines#Icon_tag_in_Desktop_Files

* install usage
--------------------------------------------------------------------------
    62  install -dD $RPM_BUILD_ROOT%{_datadir}/apps/goldendict/locale
--------------------------------------------------------------------------
  - "-D" option is not needed here.

* $RPM_BUILD_ROOT vs %buildroot
  https://fedoraproject.org/wiki/Packaging/Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS
  - Please choose one and don't use both.

* Directory ownership issue
  https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes
  - The following directories themselves are not owned by any packages.
---------------------------------------------------------------------------
%{_datadir}/apps/goldendict/
%{_datadir}/apps/goldendict/locale/
---------------------------------------------------------------------------

By the way, would you host your spec / srpm on somewhere from which
I can download them directly by $ wget -N ?
Comment 21 Howard Ning 2010-04-09 21:34:51 EDT
Thank you very much for your suggestion. 

? git based source
There is a stability bug in the 0.9 stable version. I have asked the author about this and the author suggest me to use git version. Also I have tested for months and the git version is more stable than 0.9 version.

! BuildRoot
If I delete buildroot line, the rpmlint will give me a warning about the non-presence of buildroot.

* BR
Right, it is unnecessary.

! Idendation
Right, I have added the indentation.

* desktop file
I have just follow the guide which recommend people to validate it. For the goldendict.png issue, Do I need to use a patch file to fix it?

* install usage
Yes -D is not useful.

* $RPM_BUILD_ROOT vs %buildroot
Yes, I have fixed this issue.

* Directory ownership issue
Thank you for indicating this issue. I have fixed it.

New SPEC:
http://fantastischmemo.xtreemhost.com/goldendict.spec

New SRPM:
https://koji.fedoraproject.org/koji/getfile?taskID=2107104&name=goldendict-0.9.0-7.20100307git83115ad.fc14.src.rpm

New koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=2107101
Comment 22 Mamoru TASAKA 2010-04-10 14:18:52 EDT
I have not checked your latest srpm at all, just replying to
your comments for now:

(In reply to comment #21)
> ! BuildRoot
> If I delete buildroot line, the rpmlint will give me a warning about the
> non-presence of buildroot.

- You can simply ignore this rpmlint warnings.

> * desktop file
> I have just follow the guide which recommend people to validate it. For the
> goldendict.png issue, Do I need to use a patch file to fix it?

- You can either use patch or use sed or so.
Comment 23 Mamoru TASAKA 2010-04-11 12:08:11 EDT
Okay, as I wrote in the previous comment:

- Once desktop-file-install is done, desktop-file-validate is not
  needed because if desktop file is invalid desktop-file-install also
  fails.

- If is bettter that you use "Icon=goldendict"

! BuildRoot tag is not needed (even if rpmlint warns about this).

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on my wiki page:
http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets
(Check "No one is reviewing")

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------
Comment 24 Howard Ning 2010-04-11 13:26:03 EDT
Thank you for assigning this RR. I have fixed the icon issue and deleted the unnecessary lines. 

SPEC: 
http://fantastischmemo.xtreemhost.com/goldendict.spec

SRPM:
https://koji.fedoraproject.org/koji/getfile?taskID=2108529&name=goldendict-0.9.0-8.20100307git83115ad.fc14.src.rpm

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=2108528
Comment 25 Chen Lei 2010-04-12 14:51:53 EDT
%{_datadir}/apps is not not owned by any packages in spec. Goldendict should either require kde-filesystem package or be patched to install *.qm into %{_datadir}/%{name}/translations as most Qt programs.
Comment 26 Howard Ning 2010-04-12 15:08:26 EDT
In F12, goldendict is not alone in /usr/share/app. There is a few other apps there. But in my newly installed F13, it is the only one. I will try to look into this problem.
Comment 27 Robin Lee 2010-04-12 23:51:58 EDT
Or add %{?_kde4_macros_api:Requires: kde4-macros(api) = %{_kde4_macros_api} }
before the %description section
Comment 28 Chen Lei 2010-04-13 09:46:58 EDT
(In reply to comment #27)
> Or add %{?_kde4_macros_api:Requires: kde4-macros(api) = %{_kde4_macros_api} }
> before the %description section    

/usr/share/app is defined as legacy Menu Hierarchies by freedesktop.
See http://standards.freedesktop.org/menu-spec/latest/ar01s07.html

In fedora, it's owned by kde-filesystem[   %{?_kde4_macros_api:Requires: kde4-macros(api) = %{_kde4_macros_api} }   ]
Comment 29 Howard Ning 2010-04-14 09:53:36 EDT
I have added the %{?_kde4_macros_api:Requires:
kde4-macros(api) = %{_kde4_macros_api} }.

It does not make much different. What does it do?

Also the /usr/share/apps directory is used by many other application and goldendict shouldn't own this directory.
$ ls /usr/share/apps
dcopidlng             kdeui         kssl
goldendict            kdewidgets    kstyle
kabc                  khtml         ktexteditor_docwordcompletion
katepart              kio_uiserver  ktexteditor_insertfile
kcertpart             kjava         ktexteditor_isearch
kcm_componentchooser  knewstuff     ktexteditor_kdatatool
kconf_update          knotify       LICENSES
kdbg                  konqueror     proxyscout
kdeprint              ksgmltools2
Comment 30 Mamoru TASAKA 2010-04-14 12:19:06 EDT
Adding "R: kde-filesystem" (or its variant) is sufficient.

By the way, I am still waiting for the reply to my comment 23.
Comment 31 nucleo 2010-04-14 12:48:53 EDT
If goldendict not KDE application why it should require kde macros?
May be would be better to place translations in /usr/share/goldendict or in /usr/share/qt4/translations (%_qt4_translationdir).
Comment 32 Mamoru TASAKA 2010-04-14 13:07:27 EDT
So for now R: kde-filesystem is sufficient.
kde-filesystem is noarch, just to add some directories and
won't pull in any additional dependency.
Comment 33 Howard Ning 2010-04-14 16:34:15 EDT
(In reply to comment #30)
> Adding "R: kde-filesystem" (or its variant) is sufficient.
> 
> By the way, I am still waiting for the reply to my comment 23.    

Some of my reviewed packages. I will try more.
540127
562585
578269
Comment 34 Chen Lei 2010-04-16 08:28:02 EDT
(In reply to comment #33)
> (In reply to comment #30)
> > Adding "R: kde-filesystem" (or its variant) is sufficient.
> > 
> > By the way, I am still waiting for the reply to my comment 23.    
> 
> Some of my reviewed packages. I will try more.
> 540127
> 562585
> 578269    

Using one of the following Templates to perform a review 

http://fedoraproject.org/wiki/Category:Package_Maintainers/Review_Template

Note:
You should understand all items on Review Guidelines before performing a review.
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Comment 35 Howard Ning 2010-04-19 08:54:23 EDT
(In reply to comment #34)
> (In reply to comment #33)
> > (In reply to comment #30)
> > > Adding "R: kde-filesystem" (or its variant) is sufficient.
> > > 
> > > By the way, I am still waiting for the reply to my comment 23.    
> > 
> > Some of my reviewed packages. I will try more.
> > 540127
> > 562585
> > 578269    
> 
> Using one of the following Templates to perform a review 
> 
> http://fedoraproject.org/wiki/Category:Package_Maintainers/Review_Template
> 
> Note:
> You should understand all items on Review Guidelines before performing a
> review.
> http://fedoraproject.org/wiki/Packaging/ReviewGuidelines    

Thank you. Here is another two of my reviews: 582974 582931
Comment 36 Mamoru TASAKA 2010-04-19 12:54:45 EDT
Well, umm, okay.
- Well, bug 582974 is simple, and bug 582931 is .... (srpm itself is)
  very problematic, however I will appreciate your pre-review.

Please add "R: kde-filesystem" when importing this package into
Fedora CVS.

---------------------------------------------------------
    This package (goldendict) is APPROVED by mtasaka
---------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 11/12/13, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.
Comment 37 Howard Ning 2010-04-19 23:21:26 EDT
New Package CVS Request
=======================
Package Name: goldendict
Short Description: A feature-rich dictionary lookup program
Owners: helloworld1
Branches: F-11 F-12 F-13
InitialCC:
Comment 38 Kevin Fenzi 2010-04-20 23:55:24 EDT
CVS done (by process-cvs-requests.py).
Comment 39 Fedora Update System 2010-04-21 09:22:40 EDT
goldendict-0.9.0-9.20100307git83115ad.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc13
Comment 40 Fedora Update System 2010-04-21 09:23:43 EDT
goldendict-0.9.0-9.20100307git83115ad.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc12
Comment 41 Fedora Update System 2010-04-21 09:25:29 EDT
goldendict-0.9.0-9.20100307git83115ad.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc11
Comment 42 Fedora Update System 2010-04-22 18:32:34 EDT
goldendict-0.9.0-9.20100307git83115ad.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update goldendict'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc12
Comment 43 Fedora Update System 2010-04-22 18:49:01 EDT
goldendict-0.9.0-9.20100307git83115ad.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update goldendict'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc13
Comment 44 Fedora Update System 2010-04-22 18:54:13 EDT
goldendict-0.9.0-9.20100307git83115ad.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update goldendict'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/goldendict-0.9.0-9.20100307git83115ad.fc11
Comment 45 Fedora Update System 2010-04-26 22:16:57 EDT
goldendict-0.9.0-9.20100307git83115ad.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 46 Fedora Update System 2010-04-26 22:29:58 EDT
goldendict-0.9.0-9.20100307git83115ad.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 47 Fedora Update System 2010-05-04 02:11:20 EDT
goldendict-0.9.0-9.20100307git83115ad.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 48 nucleo 2010-05-16 18:32:49 EDT
Why goldendict placed in Utilities menu?
Is Office not right place for it?
(I am using KDE)
Comment 49 Howard Ning 2010-05-16 21:04:02 EDT
The goldendict has the same group setting as stardict. Could you find a specific reason to place it under Office?
Comment 50 nucleo 2010-05-16 21:16:47 EDT
May be I am wrong but there is Dictionary in 'Additional Category' and suggested 'Related Categories' Office;TextTools.

http://standards.freedesktop.org/menu-spec/menu-spec-1.0.html
Comment 51 Howard Ning 2010-05-16 21:38:10 EDT
Please try the 0.9.0-11 version. It has added Dictionary.
If it hasn't pushed into stable, please try update-testing
Comment 52 nucleo 2010-05-16 21:47:44 EDT
I installed 0.9.0-11 version.
GoldenDict is still in Utilities menu because there is Utility in Categories
but in freedesktop standards suggested Office;TextTools instead.
Comment 53 Howard Ning 2010-05-16 21:54:58 EDT
Categories=Dictionary;Qt;Utility;
已经有Dictionary了。
Comment 54 nucleo 2010-05-16 22:08:39 EDT
The table below describes Additional Categories. The Related Categories column lists one or more categories that are suggested to be used in conjunction with the Additional Category. Note that at least one Main Category must be included in the desktop entry's list of categories. If multiple Main Categories are included in a single desktop entry file, the entry may appear more than once in the menu. If the Related Categories column is blank, the Additional Category can be used with any Main Category. 

Additional Category: Dictionary
Description: A dictionary
Related Categories: Office;TextTools

(Office is a Main Category)

Additional Category: Qt
Description: Application based on Qt libraries

I think that following to freedestop standards in desktop file should be:

Categories=Dictionary;Qt;Office;TextTools;
Comment 55 Howard Ning 2010-05-16 22:23:53 EDT
Look at default gnome-dictionary, it is:

Categories=GNOME;GTK;Dictionary;Utility;
Comment 56 nucleo 2010-05-16 22:34:24 EDT
(In reply to comment #55)
> Look at default gnome-dictionary, it is:
> 
> Categories=GNOME;GTK;Dictionary;Utility;    

May be wrong Categories there because in packaging guidelines .desktop files MUST follow the freedesktop standards:

http://fedoraproject.org/wiki/PackagingGuidelines#Desktop_files
Comment 57 Mamoru TASAKA 2010-05-17 01:14:06 EDT
Please don't use this review request which finished its review process
almost one month ago for such discuss and instead file a bug against
goldendict component then discuss there.
Comment 58 Mosaab Alzoubi 2014-09-16 19:41:52 EDT
Package Change Request
======================
Package Name: goldendict
New Branches: epel7
Owners: moceap helloworld1
Comment 59 Jon Ciesla 2014-10-20 07:47:09 EDT
Git done (by process-git-requests).

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