Cumulus / Syndic

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

Choice of CalendarLib #23

Closed Chris00 closed 8 years ago

Chris00 commented 10 years ago

May you explain the reason for switching to CalendarLib? Wall al the variations that one can find, the parsing of dates has become a mess — while Netdate.parse could handle all gracefully. CalendarLib does not seem to provide versatile parsing functions.

dinosaure commented 10 years ago

OCamlNet is too big for just parse a date. The OCaml community (or at least the Reddit OCaml community) was not very happy with this dependence.

In addition, it seems that OCamlNet is not compatible with MirageOS - unlike Uri for example.

The best solution would be to externalize this part of OCamlNet.

amirmc commented 10 years ago

To be fair, that's only one person who wasn't happy with it. From my point of view ocamlnet is already in use for ocaml.org so isn't (currently) an additional dependency. However, porting or otherwise pulling out the relevant part of ocamlnet might be a better long-term approach for everyone (though, I've no idea how much work this might involve).

Chris00 commented 10 years ago

I have submitted patches to calendar. I hope they consider merging it — and releasing an updated version would be the cherry on the cake!

kit-ty-kate commented 10 years ago

@Chris00 Why did you use the syntax for streams in your patches ? :/

dsheets commented 10 years ago

I looked for the fragment from OCamlNet but couldn't find it. Which source was moved?

kit-ty-kate commented 10 years ago

I guess it's here: https://godirepo.camlcity.org/wwwsvn/trunk/code/src/netstring/?root=lib-ocamlnet2

Chris00 commented 10 years ago

I just ported the code from OCamlNet. It is working well, I did not want to rewrite it! One could run camlp4o on it and keep the desugared version in the repo (or only in tarballs) so that the package does not depend on camlp4.

dsheets commented 10 years ago

I think parse from https://github.com/skydeck/netstring-light/blob/master/src/nldate.mlp#L208 became guess. It would be nice to mark exactly what was copied from where, who owns the copyright, and what license it's under.

Chris00 commented 10 years ago

@dsheets Actually I took it from https://github.com/skydeck/netstring-light as @samoht suggested.

Chris00 commented 10 years ago

@dsheets There is a line saying the code is copied from the above URL. Would you like something more? I renamed the function parse into guess because one may have wondered what was indeed the difference between of_string and parse. I believe this makes it more clear (but I do not have hard feelings on this).

dsheets commented 10 years ago

I guess it seems like the bounds of the copied code, the modifications to the code, the license, and the copyright holder should be in the source file. The authors of calendar should probably check that your contribution of code from another project with a (potentially) different license is compatible with their project's license.

Chris00 commented 10 years ago

@dsheets Done that.

dinosaure commented 10 years ago

Apparently Calendar can not be used on MirageOS (but it was one of the reasons for the change with OcamlNet). There are two possibilities, one can extract the compute of a date respecting the RFC and do our own API or you can simply export the date as string.

Chris00 commented 10 years ago

If these are the only two choices, I would be in favor of doing our own parsing and convenience functions — sorting posts on dates is fairly common and should not be reimplemented by every user. However, I am not so keen to have yet another date representation. Maybe one may try to contact Julien Signoles first to see whether it is possible to split his library (one OPAM package but a findlib sub-library that depends on Unix) and how he feels about it. Failing that, we indeed have to implement our own parsing and comparison functions but should provide sub-libraries to convert our representation to Calendar and ocamlnet.

Chris00 commented 10 years ago

If we re-implement the parsing function, we should maybe think of releasing a separate minimal module (with optional conversion modules) and submit patches to other libraries (to use our representation)?? It would be great to be able to agree on a common date format! One should also target odate — also uses Unix albeit through a functor. (I have not checked its parsing function.)

Chris00 commented 9 years ago

I moved and aliased all CalendarLib functions in Syndic.Date (i.e. CalendarLib is mentioned nowhere else). This way, we know what the minimal Date module has to provide.

dinosaure commented 8 years ago

See #68