Style Guide and Style Checker

[willtebbutt]

In various other repos we make use of JuliaFormatter.jl + an action to enforce a standardised style guide for each PR. See e.g. KernelFunctions.jl.

Currently this repo doesn't seem to have this, but maybe we should?

[yebai]

Seconded.

[devmotion]

It should be possible to just copy the setup in DynamicPPL (which IIRC is slightly more recent and e.g. handles merge queues as well).

[torfjelde]

We indeed should. It's been put off because it will be quite annoying for the open PRs

[torfjelde]

So once we've closed the major ones that are currently opened, I'm very pro adding the same setup as in DPPL

[willtebbutt]

So once we've closed the major ones that are currently opened, I'm very pro adding the same setup as in DPPL

Sounds entirely reasonable. Which are the major open PRs? Would be helpful to have specifics on this I think.

[mhauru]

I support this and am happy to take on implementing this. We could also add this immediately with an ignore list covering all files currently under major work. Those files could then be brought into the formatting fold as PRs are merged. This way we don't need any one moment when all big PRs would be merged.

[mhauru]

JuliaFormatter is now running in #2255. I'm also exploring other tools. Some questions:

• Do people have opinions on JET.jl? I'm trying it out now to see if it would be useful, never used it before.
• Same question for Aqua.jl.
• Do people like using git pre-commit hooks? Scripts that run before making a commit and won't let you make a commit unless the script passes. I'll make one locally to stop me from committing code that doesn't pass the formatter. If others want to use it too I could make a script for it in the repo.

[willtebbutt]

Do people have opinions on JET.jl? I'm trying it out now to see if it would be useful, never used it before.

I really like using JET. I've incorporated it into my standard unit testing in Tapir.jl -- see here. It's definitely better than just using Test.@inferred.

Same question for Aqua.jl.

Not used it, but planning to start soon -- it's really nice that it hunts down obvious method ambiguities.

Do people like using git pre-commit hooks? Scripts that run before making a commit and won't let you make a commit unless the script passes. I'll make one locally to stop me from committing code that doesn't pass the formatter. If others want to use it too I could make a script for it in the repo.

I've not used them before, but I definitely like this idea. I can definitely see myself using one which checks for formatting!

[yebai]

Do people like using git pre-commit hooks? Scripts that run before making a commit and won't let you create a commit unless the script passes.

All of these are good improvements. I am not sure about pre-commit hooks since making them run stably across a wide range of operating system choices is hard. Github checks are good enough since almost everything goes through a PR.

[yebai]

Fixed by #2255 -- feel free to open new issues / PRs for JET and Aqua.