[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


Dominik 'Rathann' Mierzejewski <rpm at greysector.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?,             |fedora-review+
                   |needinfo?(rpm at greysector.ne |
                   |t)                          |




--- Comment #21 from Dominik 'Rathann' Mierzejewski <rpm at greysector.net>  2008-12-21 06:02:05 EDT ---
(In reply to comment #15)
> (In reply to comment #14)
> > Some further comments.
[...]
> > 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.

I still think we should keep the timestamps, but I won't block on this.

> > %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)

You said copysources was a leftover, yet I still see it in the current spec.
Others that could be problematic in the future are:
%{_bindir}/get_range
%{_bindir}/make_torrent
%{_bindir}/subconv
%{_bindir}/svg_converter

I assume you will handle any file conflicts with other packages that may occur
in the future.

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

mldonkey-tools/utils? Why not. If they're useful independent of mldonkey then
it's possible. Not really important though.

Alright. There are no major issues left, so this package is APPROVED.
Thank you for the enormous effort to bring this package in line with Fedora
Packaging Guidelines. Great job.

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