[Bug 452584] Review Request: mldonkey - Client for several P2P networks

[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=452584





--- Comment #15 from Peter Lemenkov <lemenkov at gmail.com>  2008-12-07 07:16:57 EDT ---
(In reply to comment #14)
> Some further comments.

> Please fix:
> install -D -p -m 755 packages/rpm/mldonkey.sysconfig
> $RPM_BUILD_ROOT%{_sysconfdir}/sysconfig/mldonkey
> Change 755 to 644.

Done.

> BuildRequires:  m4
> BuildRequires:  autoconf
> 
> Why does it need these two?

Removed. Another one leftover.

> Shouldn't the -gui subpackage require the main package? Or can it function
> independently?

Will investigate later. I, personally, don't use gui.

> sed -i 's|\r||g' distrib/ed2k_submit/README.MLdonkeySubmit
> sed -i 's|\r||g' docs/slavanap.txt
> 
> iconv -f iso8859-1 -t UTF-8 docs/gnutella.txt > docs/gnutella.utf8 && mv
> docs/gnutella.{utf8,txt}
> iconv -f iso8859-1 -t UTF-8 distrib/Authors.txt > distrib/Authors.utf8 && mv
> distrib/Authors.{utf8,txt}
> 
> This doesn't preserve original file timestamps. Please use touch -r to do that.

There are a lot of similar issues in this spec-file. Actually, I don't think
that we need to preserve timestamps in that case, because we do change file
contents.

> # menu and pixmaps 
> install packages/rpm/mldonkey-icon-16.png -D -m 644
> $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/16x16/apps/mldonkey.png
> install packages/rpm/mldonkey-icon-32.png -D -m 644
> $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/32x32/apps/mldonkey.png
> install packages/rpm/mldonkey-icon-48.png -D -m 644
> $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/48x48/apps/mldonkey.png
> 
> I suggest a loop:
> for sz in 16 32 48 ; do
>   install packages/rpm/mldonkey-icon-${sz}.png -D -m 644
> $RPM_BUILD_ROOT%{_datadir}/icons/hicolor/${sz}x${sz}/apps/mldonkey.png
> done

Doesn't save any line (from 3 similar lines to the same 3, but completely
different :)

> %files
> ...
> %{_bindir}/copysources
> %{_bindir}/get_range
> %{_bindir}/make_torrent
> ...
> %{_bindir}/subconv
> %{_bindir}/svg_converter
> 
> These filenames seem a little too generic. Are they supposed to be run by the
> user? Could you add a "ml" prefix to them?

Yes. Some of them (exept another one leftover - 'copysources' utility) may be
used even by user. For example, make_torrent can be used even w/o main mldonkey
package. Maybe we should package them in their own sub-packages?

-- 
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.