Pervasives deprecated in 408

[anuragsoni]

The Pervasives module is deprecated in ocaml 4.08 -> https://github.com/ocaml/ocaml/pull/1605

This pull request uses dune's configurator to use the Stdlib module in ocaml versions 4.08 and above, and keeps the old pervasives module in versions older than that.

[pmetzger]

Given that this is several updates at once, could you edit the description to tell us what you've done? That makes it easier to evaluate.

[anuragsoni]

@pmetzger Thanks for the feedback. I updated the description. Although to be fair its just one update to avoid using the Pervasives module in versions 4.08 and up.

[anuragsoni]

On a sidenote, it might be nice if one of the admins can convert 3.x to master or make the development branch be the default on github. It'll make it easier to discover.

[pmetzger]

On a sidenote, it might be nice if one of the admins can convert 3.x to master or make the development branch be the default on github. It'll make it easier to discover.

@anuragsoni So the problem is that the maintainer has sort of dropped out. If you would like to take over, or even just help, I can give you commit access to this repository.

[anuragsoni]

@pmetzger I understand.

Unfortunately I won't have time to dedicate for full-time maintainer's role just yet (atleast for a couple of months). I'm not sure that a calendar/date/time library is something that should be maintained who hasn't done much work in such libraries.

What i can do though is experiment with calenderlib in my own fork, try and see if i can improve documentation to improve my own knowledge (and maybe help others looking at this). I'll make pull requests with changes are relevant and reach out once I have more time at hand to help with maintainer duties.

[pmetzger]

Even if you can't guarantee much long term, would you like access to fix the branch vs. trunk issue and to fix the build for 4.08?

[anuragsoni]

Sure i wouldn't mind updating the branches. I also made a post on discourse which might bring in some more attention from the wider community.

[XVilka]

Would be good to carve the new release after that: https://github.com/ocaml-community/calendar/issues/23

[jeromesimeon]

Would be good to carve the new release after that: #23

Does anyone have a sense of how this release will differ from the last published version on opam? I believe the last version was (?) https://opam.ocaml.org/packages/calendar/

[anuragsoni]

@jeromesimeon As far as i can tell the differences are mostly on the tooling side. The build system was switched to dune. I don't think there has been any change in the implementation.

[loxs]

@jeromesimeon @anuragsoni probably the only breaking change is this. Although "correct" IMO, it might break some legacy systems: https://github.com/ocaml-community/calendar/issues/14

[jeromesimeon]

Thanks @loxs A quick look at the commit log, I noted those?

Fixes

• Week of year was off by one in some cases (https://github.com/ocaml-community/calendar/issues/14)
• To the precision used in equality for dates (https://github.com/ocaml-community/calendar/commit/0e073eeae4ee8cb811e186755aaf959ba21e221e)
• Remove incorrect time zone bound check (https://github.com/ocaml-community/calendar/commit/49ced035269728bd7283061b31b6b60de1901596)

Build

• Support for OCaml ?? (>= 4.08.0 ?)
• Build system switched to dune
• Switch from Str to Re, for browser compatibility

[pmetzger]

@anuragsoni You have write permission now. I'd suggest that you mostly use pull requests to get things reviewed, but you can commit on your own after review. Feel free to (very carefully, and after testing locally!) fix the branch situation on your own.

[anuragsoni]

@pmetzger Thanks. @Drup Since you are mentioned in the codeowners file, if you think this patch looks okay i can merge this.

[Drup]

@anuragsoni Why didn't you used stdlib-shim ? It's literally made for that purpose.

[Drup]

Actually, looking at the code, those are not even really useful, the only thing you want is Int.compare. You don't need dune configurator just to get that.

[anuragsoni]

Thanks for pointing out stdlib-shims. I wasn't aware of it. But like you said since the only thing we need at the moment is Int.compare, i'll remove the dune configurator. I should be able to get rid of using Pervasives without that too. I'll update the PR later today when i have some time.

[anuragsoni]

The PR is now updated to no longer need dune configurator

[jeromesimeon]

Hopeful ping to any of the maintainers: This PR looks done to me?

[anuragsoni]

@jeromesimeon At this point i don't have too much motivation to continue with this PR so i'll be happy if someone else can take it through the rest of the way. At the time of the initial PR my goal was just to help out the current maintainer with something that i was hoping would be a small easy to review PR.

I think your best bet will be to make a post on discourse https://discuss.ocaml.org to see if there is someone else who will be willing to step in as a maintainer.

[jeromesimeon]

@jeromesimeon At this point i don't have too much motivation to continue with this PR so i'll be happy if someone else can take it through the rest of the way. At the time of the initial PR my goal was just to help out the current maintainer with something that i was hoping would be a small easy to review PR.

I think your best bet will be to make a post on discourse https://discuss.ocaml.org to see if there is someone else who will be willing to step in as a maintainer.

@anuragsoni Thanks for the response. That makes sense to me (and as far as I'm concerned, that PR looks done! just no one is around to merge it?). I'll try and post on discourse when I can find a moment.

[pmetzger]

I can merge something if there's consensus to merge it. That said, I already gave @anuragsoni commit privileges.

[anuragsoni]

@pmetzger I know i had the commit privileges but for projects where i'm not a maintainer i prefer when someone else goes over the change and there is a consensus to merge. I don't feel too comfortable merging my own changes for projects that I don't maintain.