Add a CI check for code formatting #151
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ld will be in the future
Schniz
force-pushed
the
add-github-action-for-refmt
branch
from
9a8f38d to
c954044
Compare
Schniz
changed the title
|
|
thomsj
reviewed
Oct 9, 2019
| @@ -76,7 +76,7 @@ | |||
| "verify-fnm-package": "node ./.ci/prepare-fnm-package.js --fail-on-difference", | |||
| "bootstrap": ".ci/bootstrap", | |||
| "test": "esy x TestFnm.exe", | |||
| "fmt": "bash -c 'refmt --in-place {library,executable,test}/*.re'", | |||
| "fmt": "bash -c 'esy @fmt refmt --in-place ./*/*.re'", | |||
There was a problem hiding this comment.
Do I need to do something to get this to work? Currently I get:
error: project is not installed, run
esy install
Is @fmt a project, and if so, how do I install it?
|
Good call. Running `esy @fmt install` will do the trick. Maybe it should be
a part of the program. Once the PR I have opened to Reason will be merged
this new project will be removed.
On Wed, 9 Oct 2019 at 16:58 thomsj ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In package.json
<#151 (comment)>:
> @@ -76,7 +76,7 @@
"verify-fnm-package": "node ./.ci/prepare-fnm-package.js --fail-on-difference",
"bootstrap": ".ci/bootstrap",
"test": "esy x TestFnm.exe",
- "fmt": "bash -c 'refmt --in-place {library,executable,test}/*.re'",
+ "fmt": "bash -c 'esy @fmt refmt --in-place ./*/*.re'",
Do I need to do something to get this to work? Currently I get:
error: project is not installed, run esy install
Is @fmt a project, and if so, how do I install it?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#151?email_source=notifications&email_token=AAPVU5D2USRAJTVV2EJQXXTQNZHXNA5CNFSM4I6MQBQ2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCHOWKSQ#pullrequestreview-299722058>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPVU5FXOQDP5KSQ7P2TWXTQNZHXNANCNFSM4I6MQBQQ>
.
--
Sent from my iPhone
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a GitHub action (🎉) that checks for the latest refmt syntax.
This is done thanks to @thomsj which brought up the problems with formatting in the repo! 👏 #105 (comment)
The current implementation is not ideal: it installs
esyand then installs OCaml and@esy-ocaml/reason. A better way to do it is to distributerefmtas a standalone binary from the latest Reason release. This will be fast to install and therefore fast to integrate with CI.An approach to do so is to add an artifact to the CircleCI runs, and then use something like https://circleci-artifacts.now.sh to fetch the artifact, extract it and profit 💰
In the meantime, it takes <4m to run it, which is much faster than the Azure Linux build, not to mention the Windows one.