search

alpmestan / servant

Moved to http://github.com/haskell-servant
50 stars 1 forks source link

implement BaseUrl to allow https for Clients #28

Closed soenkehahn closed 10 years ago

soenkehahn commented 10 years ago

Please review and merge.

soenkehahn commented 10 years ago

Fixes #27.

soenkehahn commented 10 years ago

Please wait, still working on this.

soenkehahn commented 10 years ago

Ok, now this would be ready for merging (pending travis approval). The last four commits are rather meant as propositions as basis for discussion. cc: @alpmestan @jkarni

soenkehahn commented 10 years ago

Ideally, we would have two Utils-like super-modules:

Do you think this division is important? If yes, any ideas for good names?

jkarni commented 10 years ago

Something that's related came up, namely that we are starting to have invariants that can only be guaranteed if certain things aren't exported. So I propose an Internal dir . Then, unless it you think it wrongly indicates a connection between the two, we could have the first Utils be inside of Internal, and the second outside.

soenkehahn commented 10 years ago

I thought about that, too, but I don't think it's a perfect solution, as some things from these internal utilities modules will end up in the public API. (E.g. FromText.) And that's confusing: we have an Internal module and encourage people not to import that and then we re-export stuff from it in the normal external API. (But maybe that's not that problematic.)

(I do however think that an Internal module is the right solution for your problem with invariants.)

What do you think about having Utils (for Links and ApiQuasiQuoting) and CoreUtils (for BaseUrl, Req and Text)?

jkarni commented 10 years ago

LGTM, modulo whatever we decide about the module-renaming. I think the organization you picked is good though - if we need to, we can change it again later.

jkarni commented 10 years ago

Right - Internal may be best left for things that should never be imported in client code. Maybe Common (for BaseUrl etc) vs Utils? Either sound good.

soenkehahn commented 10 years ago

Yes, Common, I like that. Will update this PR.

alpmestan commented 10 years ago

Internal for things that are not meant to be exposed sounds good. Common should be fine for the other, unless it starts getting bloated, in which case we may want to subdivide further. I do hope however that most of the time people won't have to worry about this and will just import Servant :)

soenkehahn commented 10 years ago

Agreed.

soenkehahn commented 10 years ago

Ok, finally good to merge.

alpmestan commented 10 years ago

Can I merge safely now?

soenkehahn commented 10 years ago

Yes, should be good.