[Bug 397211] Review Request: redet - Regular expression development and execution tool

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

Summary: Review Request: redet - Regular expression development and execution tool


https://bugzilla.redhat.com/show_bug.cgi?id=397211





------- Additional Comments From pertusus at free.fr  2007-11-25 08:11 EST -------
(In reply to comment #3)
> (In reply to comment #2)
> 
> > I think that in the make install line, INSTALL="%{__install} -p"
> > is not useful, and that you should add 
> > BINDIR=%{_bindir}
> 
> BINDIR is not necessary since the Makefile sets it correctly using PREFIX. But
> it does not do that with MANDIR, and hence it had to be set explicitly.

I disagree, here. With the default fedora macros, %{_bindir}
is %{_prefix}/bin, redet uses PREFIX=%{_prefix} and appends bin,
so it works.

However, the macro that should be used for BINDIR is %{_bindir},
it is not %{_prefix}/bin. So BINDIR should be set to %{_bindir},
to cope with redefinitions of %{_bindir}.

> > Also since you try hard to keep timestamps, I think it would be better
> > to keep them despite the sed.
> 
> How do I do that with sed? I am not very familiar with it. 

I suggest something along:
sed --expression \
  's|set NonBinPath \[file join /usr local share Redet\];|set NonBinPath \[file
join /usr share Redet\];|' redet.tcl > redet.tcl.tmp
touch -r redet.tcl redet.tcl.tmp
mv redet.tcl.tmp redet.tcl


> > It would be nice to provide the manual. It is even 
> > called from the program, from %{_datadir}/Redet/Manual/Manual.html
> > This would certainly mean using the redet-full-8.23.tar.bz2
> > tarball.
> 
> The manual is 4.9M in size and was historically provided by the redet-doc
> package. I have inherited it too, and will soon submit a review. Since the
> manual is so big in size, I do not want to put it in the main package and burden
> users with such big downloads.

You are right. However, since it is quite hard to understand the tool
without the manual, I suggest mentionning the redet-doc package
in the %description, like

The documentation for this package is in %{name}-doc.


Also I suggest hardcoding the version in Patch0 to show that this
was the version the patch was introduced in.

-- 
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, or are watching someone who is.