On 29/12/2012 8:16 p.m., Eliezer Croitoru wrote:
Thanks for all the help until now.
This PATCH is in sync with trunk which comes after the last preview.
I tried my best until now in the small windows of time I have for the
project with hopes for the best.
This no longer a PREVIEW since most of the code was reviewed couple
times and was tested in a small production testing environment for
couple weeks with a great success.
I left couple FIXME and TODO in the code.
One question I had regarding the documentation\comments:
I have seen in the code couple places where the first line of a
comment was out-of-line from the comment start like "/**\n * text\n
*/" and others where the first test is inline with comment start and
wasn't sure how much it's important.
I have tried to use in most of my code the "/**\n" since I think it's
more readable.
Is there a preference about it?
Up to you. Text on the first line is used as a brief description when
the full text may be long.
I have decided to leave the store_id string variable null in cases
it's not needed which allow to use the store_id.undefined().
This adds the flexibility of a basic true\false check when needed later.
Okay.
While testing some helpers I have seen a bug when using concurrency
which I will debug later after committing the working StoreID to trunk.
TESTS: I tested this patch and found some bugs related to helper code
and I am looking for other tests.
cases:
- regular ok reply with store-id=value(not empty) = works.
- reply with empty store-id key pair = getting some assertion problem.
- ok reply with the original url = works and wont set the store_id
value.
- simple PURGE request from squidclient = erases the object.
havn't tested yet?
- broker helper replies(drop me some if you have in mind).
- reply with k-v other then store-id.
- reply with too many spaces between parts.
- htcp related tests.
- icp related tests.
Regarding back-porting the code into 3.2 branch:
What do think? There is quite a different in the design of the code
in 3.2 vs trunk.
I am probably releasing 3.3 as stable next month. In which case it will
not be worth the effort.
The next thing after this patch is adding ICAP the option to respond
with some StoreID for a request.
How do I even start on that?
and a case I had in mind is what to do when Store-ID is used in ICAP?
do we allow second helper?
if so what and how do we send to the helper since it comes after the
ICAP REQMOD service ?
Logically both are working on whatever the storeId currently is to
produce no-change or a new one. So I think we should let the helper be
passed the storeID from ICAP. Leaving it up to the admin whether to use
two or one, and when each operates.
I have reviewed the patch and code couple times but I still might not
see something.
If someone have some scripts of basics code\syntax checks I will be
more then happy to have them in order to check any future code related
the project.
./scripts/sourcemaintenance.sh is the main one. It requires a secific
version of astyle to do full syntax checks but some checks are always run.
Amos
