Cumulus / Syndic

RSS and Atom feed parsing
MIT License
34 stars 13 forks source link

fix UTC offset parsing #60

Closed pwbs closed 9 years ago

pwbs commented 9 years ago

fix https://github.com/Cumulus/Syndic/issues/59

modified: lib/syndic_date.ml

dinosaure commented 9 years ago

I'll watch this PR tonight. But I think there is no problem.

pwbs commented 9 years ago

OK, thank you @dinosaure :)

If you merge it, could it be released soon in opam? I'm asking because I'm currently opam-pinning my fork and I'd like to limit opam-pins as much as possible, especially when it's for only 6 bytes.

dinosaure commented 9 years ago

OK, I watched the problem comparing with Python and I fixed using Calendar.Time_Zone instead of Time.add. This works fine, we expect a good result. But it's not the same as Python:

>>> strict_rfc3339.timestamp_to_rfc3339_utcoffset(time.mktime(rfc822.parsedate("Wed 21 Oct 2015 06:00:13 +0200")))
'2015-10-21T05:00:13Z'
utop # Syndic_date.of_rfc822 "Wed, 21 Oct 2015 06:00:13 +0200" |> Syndic_date.to_rfc3339;;
- : bytes = "2015-10-21T04:00:13+00:00"                                                                                                 

I commit and release tomorow if nobody explain that bug (maybe it's just false because I use time.mktime in Python). Thanks for all !

pwbs commented 9 years ago

I'm not familiar with Python. Anyways, I don't understand how 06:00:13 +0200 is supposed to be equivalent to 05:00:13Z. I'm inclined to think that Python can't be trusted...

dinosaure commented 9 years ago

Close by #60.