[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 #4 from Stefan Schulze Frielinghaus <stefan at seekline.net>  2009-09-24 07:51:11 EDT ---
(In reply to comment #3)
> Tabs aligned properly. Just you use another tab with (see first post).

I red your first comment and just wanted to encourage you to use "normal" tabs
or 8 whitespaces. I guess this not a big problem but why do you want to go a
different why than most of the other packages? In a very strict sense this may
fell under
https://fedoraproject.org/wiki/Packaging/ReviewGuidelines#cite_note-5

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

Oh. Haven't noticed that before. Thanks.

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

Then why not write a patch and wrap it with a conditional for 64bit like you
already mentioned? This would solve it. I'm not that familiar with this package
but what does upstream say about this? Maybe they have the same problem?

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

Since it is a library the example files may help developers to build against
it. But again this is not a problem and I guess up to the packagers freedom.

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