[Bug 521909] Review Request: ne7ssh - SSH Library is a Secure Shell client software written in C++

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





--- Comment #3 from Pavel Alexeev (aka Pahan-Hubbitus) <pahan at hubbitus.info>  2009-09-24 07:15:47 EDT ---
(In reply to comment #2)
> A few comments about the spec file:
> 
> - Tabs aren't aligned properly
Tabs aligned properly. Just you use another tab with (see first post).

> - I'm not sure about this comment:
> # It still required for EPEL5
> BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>
> BuildRoot is required on Fedora too or do I miss something?
Yes BuildRoot is obsoleted for Fedora.
http://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

> - What about this comment (line 15)?
> #Requires:
This is atavism. Deleted.

> - You should write a patch instead of using sed here:
> # Hack to correct install on 64-bit systems.
> sed -i ...
> 
> sed will return 0/no error if it substitutes something or even does nothing but
> patch will fail if it cannot be applied. With a patch you are on the save side.

Yes, there sed behavior is preferred.
1) We must be conditionally apply patch only on 64-bit systems.
2) sed always replace in this string, even if "lib" to "lib". So, it is normal.


> - The example package only contains 7 CPP files. Maybe you want to pack them
> into the main package as docs too? What do you think?
For what? Especially when work to split it was done.

> - I guess it is a typo in the Changelog
> Fix installation on 54-bit systems.
> 
> should be
> 
> Fix installation on 64-bit systems.  

Off course. Thank you.

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