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=682666 Mohamed El Morabity <pikachu.2014@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |pikachu.2014@xxxxxxxxx --- Comment #2 from Mohamed El Morabity <pikachu.2014@xxxxxxxxx> 2011-04-14 21:19:40 EDT --- Since I *really* need this tool, I'd be glad to review it. Some comments: * your patch could be *really* simplified: - the flex package provides also lex, which is a symbolic link to flex ; - in the same way, libl.a is provided by the flex-static package and is a symbolic link to libfl.a As a result, modifying the LEX and LEXLIB variables in your patch is useless. You could even skip the  make install ...  instruction in %install in your .spec, and manually install the binary and the man page, like below: %install rm -rf $RPM_BUILD_ROOT install -Dpm 755 detex $RPM_BUILD_ROOT%{_bindir}/detex install -Dpm 644 detex.1l $RPM_BUILD_ROOT%{_mandir}/man1/detex.1l (notice the -p option of install to preserve timestamp during installation). * detex is not compiled using the Fedora standard flags ($RPM_OPT_FLAGS). It appears clearly in compilation logs. Moreover the generated *-debug package is unusable. You must set the CFLAGS when calling make: %build make CFLAGS="$RPM_OPT_FLAGS" You can even add to CFLAGS the  -DNO_MALLOC_DECL  option you enabled on your patch; by this way, the patch is useless and can be removed from your package: %build make CFLAGS="$RPM_OPT_FLAGS -DNO_MALLOC_DECL" * The 1l section for man pages is sometimes intended for programs installed in /usr/local. This may be the reason for the man page to be suffixed  1l Â. Whatever the explanation, it's not correct. I suggest you to rename the man page to  detex.1 Â: in %install, simply: %install rm -rf $RPM_BUILD_ROOT install -Dpm 755 detex $RPM_BUILD_ROOT%{_bindir}/detex install -Dpm 644 detex.1l $RPM_BUILD_ROOT%{_mandir}/man1/detex.1 (dont forget to fix it also in %files section of your .spec). As a result, you should also patch the man page and replace each occurence of  1L  to  1 Â. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review