OCamlformat all of the things! #477
Conversation
| @@ -0,0 +1,13 @@ | |||
| #! /usr/bin/env bash | |||
There was a problem hiding this comment.
Needs a . ./scripts/support/assert-in-container $0 $@
|
@pbiggar It seems to work fine, and I get no actual compiler errors or warnings -- so best guess is pygments' parser doesn't like it. I'm having a google around now to see if there's any other talk of OCaml multi-line string escaping weirdness. |
|
I'm a little bit concerned around the 3 open PRs - I think the rebases will be really bad. Is there a simple bash to redo all the code formatting? If so I can run it at the weekend or something. |
|
The sql in ea52d69#diff-90d6afcb7795c268b727e72e43d372f0R110 is a problem. I really dont want to make sql harder to read/write, given how error prone it already is. Is there a tag to disable ocamlformat for certain blocks or similar? |
IanConnolly
force-pushed
the
chores/enable-ocamlformat
branch
from
eb8d16b
to
19ad246
Compare
|
@pbiggar couple of things:
|
| Log.infO "Libexecution" ~data:"Initialization Complete"; | ||
| has_inited := true | ||
|
|
||
| else Caml.print_endline "Libexecution Initialization Begins" ; |
| @@ -0,0 +1 @@ | |||
|
|
|||
There was a problem hiding this comment.
What's it doing here?
(Also, why do we have an empty file?)
| | F (_, PVariable c), F (_, PVariable n) -> | ||
| renameVariable c n e | ||
| | _ -> | ||
| (* | ||
| * For now, only allow if all variables in the replaced |
| | Some (c, n) -> renameVariable c n body | ||
| | _ -> body | ||
| match | ||
| pairCurrentAndNew |
There was a problem hiding this comment.
Are we able to not split match exprs?
| | FeatureFlag (msg, cond, a, b) -> | ||
| [PFFMsg msg; PExpr cond; PExpr a; PExpr b] | ||
| | Match (matchExpr, cases) -> | ||
| (* We list all the descendents of the pattern here. This isn't ideal, | ||
| (* We list all the descendents of the pattern here. This isn't ideal, |
There was a problem hiding this comment.
One of the things I liked about janestreet was the lack of this indent. Is it possible to kill the indent?
There was a problem hiding this comment.
nvm, I think I'll just get used to this one.
|
Perhaps the path here is to merge this:
and the we can turn it on later (I'm not sure when exactly later is though, given there's always work in progress) |
IanConnolly
force-pushed
the
chores/enable-ocamlformat
branch
from
19ad246
to
d9a7479
Compare
|
@pbiggar two PRs are merged, and this is rebased |
This is definitely a PR you want to view commit-by-commit, because the final commit is a huge reformatting change you want to ignore.
The main bits here are want to add a
.ocamlformatconfig file in the repo so that all of our editors use the correct configuration, and CI enforcement of the use of OCamlformat. The setting up of editors was already added by @ismith in an earlier PR (#389).The merge plan here is simply: merge this, and CI will immediately enforce OCamlformat usage. Everyone should then set up their editors to do so. I'd be happy to help people with that 1-1 via slack and follow up with README changes if necessary.