[Bug 581619] Review Request: mlterm - Multi Lingual TERMinal emulator on X

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

--- Comment #16 from Hicham HAOUARI <hicham.haouari at gmail.com> 2010-06-10 09:38:00 EDT ---
(In reply to comment #14)
> > 1) Please provide a direct link to the spec everytime you update the package.
> 
> no problem, I'll do that
> 
> > 2)The %{_libexecdir}/* should be replaced IMHO with the corresponding files separately.
> 
> is there a reason for that ? they are files that might change from version to
> another
that discards the whole point of explicitely specifying files in the %files
section, please fix that
> 
> > 3)You forgot to update the changelog.
> ok
> 
> > 4)BuildRequires : gtk+-devel, better put gtk2-devel
> 
> we can't do that, the config window of mlterm is a gtk1 application
> 

The build i have done on comment 14 is not using gtk1

> > 5)Patch should be upstreamed ( put a link to the corresponding upstream
> bugzilla bug report )
> 
> if you are referring to your patch in #11 then it just -lx11 to build option,
> it could be missed because of many reasons, and BTW it used to build in F11 and
> F12 without this patch, so if the upstream is using fedora<13 he won't be
> convinced that there is any problem

All sane upstreams projects will understand the importance of the change in
Fedora policy about dso linking

> 
> 
> > 6)As mentioned above, change "sed -i -re 's/#ifdef  HAVE_ISASTREAM/#ifdef
> HAVE_STROPTS_H/' kiklib/src/kik_pty_streams.c" to a patch and upstream it. Same
> for fribidi-config
> 
> the fribidi-config patch is no longer needed that's why it's commented (it was
> needed because the upstream used to use fribidi-config instead of pkg-config
> which is in fedora)
> 
> and the sed change is needed to build it, try building without it.
> the function it uses "isastream" is from stropts.h I guess both are not in
> fedora so HAVE_STROPTS_H is equivalent to HAVE_ISASTREAM
> 
> http://www.opengroup.org/onlinepubs/009695399/functions/isastream.html    
Then the code should check for either one of them.

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