DEEPDIP-project / CoupledNODE.jl

Apache License 2.0
2 stars 0 forks source link

Precommit: formatting and creation of notebooks/markdown #25

Closed APJansen closed 6 months ago

APJansen commented 6 months ago

This PR adds precommit hooks for JuliaFormatter and notebook creation, as well as a check on the formatting in the CI.

See here for instructions on how to set it up.

JuliaFormatter hook

I have used the "official" JuliaFormatter hook following this.

It is not as nice as what I'm used to in python with black, in that a. it checks all files rather than only those that have been modified, and b. it doesn't list the files it had to format.

Not a big issue, as long as everyone uses it the only files that it has to modify should be ones you changed, and if you want to see exactly which, you can just do a git status.

notebooks hook

I didn't find any existing hook for this, so I created one that just calls the julia script (changing the the overwrite setting to true). It seems to work fine for me.

JuliaFormatter CI

Here I also used the official one. It seems overly complicated to me, going through some code checker service reviewdog. Also it doesn't seem to be working, in that it passed on this PR before I applied any formatting.

Remaining issues

APJansen commented 6 months ago

I had to run the formatter in the CI manually as well (i.e. without using the official action), but now it works fine. here is an example of what you see on failed formatting: all the changes it made.

APJansen commented 6 months ago

I rebased this on main and squashed everything. The only thing that's not working is that it considers also the files in examples/coupling_functions as notebooks, but that's probably best fixed in a separate PR changing the file structure.

APJansen commented 6 months ago

sorry one sec, didn't actually rebase on latest main.

APJansen commented 6 months ago

Can you both check if you can set up pre-commit following the instructions in CONTRIBUTING.md, and if when you run pre-commit run --all-files nothing changes? Just to be sure it's correctly reading the settings for everyone.