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

Pre-commit hooks #7

Open
dc-mak opened this issue Mar 14, 2021 · 0 comments
Open

Pre-commit hooks #7

dc-mak opened this issue Mar 14, 2021 · 0 comments

Comments

@dc-mak
Copy link
Collaborator

dc-mak commented Mar 14, 2021

Let's check that we can build, test and format cleanly in a pre-commit hook.

If you want to ignore warnings when developing, we should add a make target/flag that passes the --profile release option to dune.

https://github.com/CuiCui66:
I'm more in favor of leaving the default profile dev without warnings, and then create a --profile commit that is called by the pre-commit hook and that has the warning as error thing. This is for multiple reasons:

  • The compiling command you call the most of time (and by hand) is the one you use when you developing new code/experimenting, So it's good if that's the default (and the profile is named dev).
  • The --profile release is not intended at all for removing warnings but for releasing (as the name indicate) and therefore has (or will have) more release related effect like not passing the -g option, disabling assertions, compiling with -O3 on the flambda compiler, ...

What I suggest is something like:

(env
 (dev (flags (:standard -warn-error -A)))
 (commit (flags (:standard -warn-error +A)))
 (release (flags (:standard -noassert)))
)

And then the pre-commit hook can call dune with --profile commit

And last but not least, I'm not sure this should be done at the commit level. I often do commits with name "WIP" and random code that do not even compile, then I erase/squash them and make a clean commit to send upstream, so this should probably be at PR level (Checking that the code builds without warning, tests and formats properly)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant