On Tuesday 12 May 2015 22:05:25 Hubert Tarasiuk wrote: > Hello Everyone, > > I have prepared conditional GET requests support for Wget (specifiaclly > If-Modified-Since header). > > Following steps were taken (each is one patch): > 1) local file timestamp support added to testenv > 2) 304 return code added to testenv (ie. do not send response body when > 304 was requested for given file) > 3) Test-condget.py - unit test for conditional requests > 4-5) "--condget" option handled in src/http.c > 6) Short description in user manual > > I would be very pleased if you could review my code and maybe point out > some mistakes or any other comments/suggestions. Maybe you have ideas > about some improvements to it? > > In case you prefer to view it on Github, here is a link: > https://github.com/jy987321/Wget/commits/master-hubert
Hi Hubert, nice to see your work... it looks very good to me. Just from a quick first glimpse, there are a few small points: - please avoid abort() (found in time_to_rfc1123()). The function returns RETROK on success but calls abort() on failure. This might end up in a frustrating user experience. On error, you could fall back to not using if- modified-since at all, fall back to using HEAD or fall back to use a time value of 0 (corresponding to 1.1.1970 00:00:00). Or whatever you think is appropriate. - typo in Test-condget.py (usiong instead using) - have a look if your IDE/editor supports spell checking. - it would be nice if your test case tests all variants of HTTP-date (= rfc1123-date | rfc850-date | asctime-date) Regards, Tim
signature.asc
Description: This is a digitally signed message part.
