search

rgrinberg / opium

Sinatra like web toolkit for OCaml
MIT License
755 stars 67 forks source link

Kick out stuff that depends on unix into its own findlib package #22

Closed rgrinberg closed 5 years ago

raboof commented 9 years ago

+1 :)

I'd be willing to help but I'm afraid I'd require some hand-holding as I'm not too familiar with OCaml yet.

raboof commented 9 years ago

So does this mean Opium.App should become a functor with a Cohttp_lwt.Server as parameter?

rgrinberg commented 9 years ago

Hi @raboof, thanks for the interest. Looks like I've missed your first comment.

At first glance, yes, functorising over Cohttp_lwt would work. But it's a bit of a brute force approach. At the end of the day, an opium app is approximately Rock.Request.t -> Rock.Response.t - none of that is unix dependent. So the same Opium_rock.App.t should run on unix/mirage without modification in principle. Nevertheless, I've let some unix dependencies creep up:

To address the points above we need to provide 2 opium entry modules

Opium_rock will also need to be stripped of unixisms and extended by Opium_rock_unix and Opium_rock_mirage (providing the platform dependent run function). After that it should be wrapped by the appropriate Opium*.Std module as Rock.

In summary, I don't see functors being necessary to do this separation. Most of the work will just be splitting modules and re-assembling them in the appropriate Std modules.

Let me know if you have questions. This could be a lot to chew down if you haven't done much OCaml.

raboof commented 9 years ago

Would Opium_mirage.Std contain anything Mirage-specific? If not, could we call that Opium.Std ('bring your own Cohttp_lwt.Server'), and have an Opium_unix.Std to make unix-specific things available?

rgrinberg commented 9 years ago

@raboof I think it's sensible to include mirage dependent stuff there. I'd prefer if the base was Opium_kernel though and leave Opium.Std as unix. For the simple reason that the vast majority of users today only care about that.

raboof commented 9 years ago

OK, shifted things around a bit in #36 - I wasn't sure where to put start and run_command module-wise.

anuragsoni commented 5 years ago

Closing this as opium_kernel doesn't have any unix dependancies, and i'm still focusing on just keeping Opium.Std focussed on unix