------- You are receiving this mail because: -------
You are on the CC list for the bug.
http://bugs.exim.org/show_bug.cgi?id=1295
--- Comment #7 from Philip Hazel <ph10@???> 2012-10-03 19:15:57 ---
On Tue, 2 Oct 2012, Zoltan Herczeg wrote:
> I think the patch is quite good in overall, although I just skimmed
> through it (no more time at the moment). Really nice job! I am sure
> Philip also want to take a look before you land it.
I agree, it is a great job! I'm so impressed that you have worked on all
the support scripts and documentation as well as the code. I too have
just skimmed it ... I agree with Zoltán's comments and have just a few
very, very minor points that I noticed:
(1) In CMakeLists.txt you have "Either PCRE_BUILD_PCRE8, PCRE_BUILD_PCRE16
or ....". Picky English-speakers would prefer "At least one of"
instead of "Either" (as "either" means "one of two").
(2) Why does the patch remove item 17 (Fix JIT tests) in ChangeLog?
(3) Another picky language thing: in NON-AUTOTOOLS-BUILD perhaps "The
pcretest program can be linked with any of the 8-bit, 16-bit ..." might
be a bit clearer if it said "can be linked with any combination of the
8-bit, 16-bit..." so that it is clear that it can be linked with more
than one of them.
(4) In the pcre32.3 man page Zoltán is rightly credited with the work
for 16-bit support; Christian, you should also be mentioned for the
32-bit work. It was not a small job.
I think you should go ahead and commit the patch. I have not got any
changes outstanding at the moment, and don't plan on any in the next few
days. If Zoltán's work is also up-to-date, it seems like a good time to
get the patch into the trunk. I will then play with it and and generally
take a look around the documentation etc to see if there is anything
else that I think needs doing.
An excellent job!
Philip
--
Configure bugmail:
http://bugs.exim.org/userprefs.cgi?tab=email
