ThinkOpenly / sail

Sail architecture definition language
Other
11 stars 12 forks source link

Dune format #21

Closed snapdgn closed 6 months ago

snapdgn commented 6 months ago

Format the json backend with dune fmt.

NOTE: The rest of the sail already uses dune fmt / ocamlformat. So we can directly use dune fmt to format the whole source code.

snapdgn commented 6 months ago

I was looking into writing a workflow for Auto formatting the code, but we already have one: https://github.com/ThinkOpenly/sail/blob/json/.github/workflows/formatting.yml, enabling this should do the trick.

ThinkOpenly commented 6 months ago

This commit keeps popping up unexpectedly:

[JSON]: Implement support for optional operand

Given the pervasive nature of the changes here, you might need to reset, rebase, rerun the reformat, and force push. Let me know if I can help.

snapdgn commented 6 months ago

Yeah, I noticed. I forgot to rebase with the upstream. Will cleanup as soon as possible.

snapdgn commented 6 months ago

@ThinkOpenly , I think this is ready, the formatting workflow ran smoothly (as evident in the checks below), and I've already rebased as well.

Build workflow is failing, I'll look into it and provide a fix through a separate PR.

ThinkOpenly commented 6 months ago

Build workflow is failing

It appears to be the same "Test Coverage" action that has failed before this PR, and failing in the same way. I'm asserting that this PR didn't introduce a new build failure, would you agree?

snapdgn commented 6 months ago

It appears to be the same "Test Coverage" action that has failed before this PR, and failing in the same way.

Yes. But before this both the Test Coverage and Check formatting was failing. Formatting is fixed now.

Yes. This PR didn't introduce a new build failure. It was happening before too.

joydeep049 commented 4 months ago

I was looking into writing a workflow for Auto formatting the code, but we already have one: https://github.com/ThinkOpenly/sail/blob/json/.github/workflows/formatting.yml, enabling this should do the trick.

I just finished creating a Formatter for Ocaml too! But then realised we already have one. But does our formatter in formatting.yml focus too much on the standards enforced by ocamlformat ??

@ThinkOpenly @snapdgn

ThinkOpenly commented 4 months ago

does our formatter in formatting.yml focus too much on the standards enforced by ocamlformat ??

I'm not sure how to answer your question. Are you observing behavior that is concerning?