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@xxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review