[Bug 545405] Review Request: goldendict - A feature-rich dictionary lookup program

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=545405

Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mtasaka at ioa.s.u-tokyo.ac.jp

--- Comment #20 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp> 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 ?

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.