Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ocamlformat #7

Closed
pbiggar opened this issue Mar 21, 2019 · 5 comments
Closed

Add ocamlformat #7

pbiggar opened this issue Mar 21, 2019 · 5 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@pbiggar
Copy link
Member

pbiggar commented Mar 21, 2019

We should ocamlformat using:

profile = sparse
break-string-literals = never
escape-strings = preserve

This would be a check in CI (once it's set up) to verify that everything is properly formatted.

@pbiggar pbiggar added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 23, 2019
@pbiggar
Copy link
Member Author

pbiggar commented Apr 1, 2019

This is getting serious. We should:

  • ocamlformat every file
  • check that ocamlformat works on CI

@wpcarro
Copy link
Contributor

wpcarro commented Jul 26, 2019

I don't mind giving this a go. I'll try and PR something today.

@Dean177
Copy link
Collaborator

Dean177 commented Jul 26, 2019

@wpcarro I actually started this aaages ago: https://github.com/Dean177/tablecloth/tree/ocamlformat

But had trouble getting it working with the bucklescript side of things, that might be a good starting point. (Maybe just to check it works for the native side and submit that first)

@wpcarro
Copy link
Contributor

wpcarro commented Jul 26, 2019

@Dean177 thanks for chiming in. I'm testing out a configuration that I'm hoping should work here: #70. I'm expected the CI integration to fail at first, but I wanted to get something up to get some feedback.

Would love your review if you don't mind! Might not be worth looking at until I sort out the build issue. Should hopefully resolve that sooner than later.

@wpcarro
Copy link
Contributor

wpcarro commented Aug 2, 2019

Should we close this now because of #70 ?

@pbiggar pbiggar closed this as completed Aug 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants