andreas / ocaml-graphql-server

GraphQL servers in OCaml
MIT License
624 stars 60 forks source link

ocamlformat #173

Closed naartjie closed 4 years ago

naartjie commented 4 years ago

Would you accept a PR to add ocamlformat to the project? If so, any preference for profile?

I was thinking of just adding the project files, and @andreas could run dune build @fmt --auto-promote. I wouldn't want to be the main committer.

Anyways, let me know if you'd be keen, if so, I can do a couple of disposable PR's with the different profiles, just to preview what the code would end up like with each of them.

andreas commented 4 years ago

I've been wanting to adopt ocamlformat for a while, but never got around to investigating the format options. I would imagine going for conventional or ocamlformat. If you want to help out investigating and trying something out, I'd appreciate it 😄

andreas commented 4 years ago

Thanks for creating those PRs! I'm inclined to use conventional. Do you happen to know if there's a good way to enforce the ocamlformat style with CI, i.e. fail the build if the formatting does not conform?

cem2ran commented 4 years ago

@andreas you could consider adding this as a pre-commit hook?

andreas commented 4 years ago

@andreas you could consider adding this as a pre-commit hook?

Yeah, I guess that could be a start.

anmonteiro commented 4 years ago

Do you happen to know if there's a good way to enforce the ocamlformat style with CI, i.e. fail the build if the formatting does not conform?

This is what I do in H2:

https://github.com/anmonteiro/ocaml-h2/blob/a0a916a4b7b21b9fa3abe1a89c946207f15c270a/.circleci/config.yml#L108-L109

naartjie commented 4 years ago

+1 for @anmonteiro's method of failing the build. H2 is using CircleCI, would just need to figure out how to run that check in travis.

yawaramin commented 4 years ago

Looks like this is closed by #177

andreas commented 4 years ago

Indeed, thanks!

naartjie commented 4 years ago

Thanks @yawaramin, I was keeping this open in order to take care of the CI check, but it's better to open a new issue for that 👍