cholla-hydro / cholla

A GPU-based hydro code
https://github.com/cholla-hydro/cholla/wiki
MIT License
65 stars 32 forks source link

Introducing pre-commit configuration to call clang-format #404

Closed mabruzzo closed 3 months ago

mabruzzo commented 3 months ago

Overview

This introduced a basic configuration file for the pre-commit software that calls clang-format.

In general, pre-commit is a tool written in python that was originally designed to be use locally (any user of cholla is free to do that). In the original design, it was intended to be used to easily attach varying "lints" to git's "commit-hooks". More recently, pre-commit has become popular for continuous integration. In principle, pre-commit can be used with any arbitrary continuous integration provider (whether it's Jenkins, GitHub Actions, CircleCI, etc.).

We are interested in using pre-commit.ci, which is a continuous integration system designed around pre-commit.

yt actually makes use of pre-commit.ci

What does this PR do?

Specifically, this PR adds the configuration file for pre-commit. I have configured that pre-commit file to use clang-format (with the precise version that matches the wiki -- 17.0.1).

I also made a tweak to a file flagged by a run of pre-commit. I'm a little worried about that... But, if the existing CI test suite succeeds, then I think we are probably fine. (Clang-format will continue to run as part of Jenkins, so will know if any disagreements arise -- there really isn't any risk for Cholla). EDIT: I'm no longer worried -- the GitHub action seems to be totally fine with this change in file formatting. Plus, it looks like Jenkins was successful at applying clang-format

What we need to do after this PR is merged

We will need to go to the pre-commit.ci and enable it for the Cholla repository. (I'm happy to try to do that, but I may not have appropriate permissions -- so Evan may need to do that)

mabruzzo commented 3 months ago

Here's an example of a comment in a pull request that I made to yt a while back, where one of the maintainers used pre-commit.ci to reformat my python-code

evaneschneider commented 3 months ago

Looks like everything passed this times, so I'll go ahead and merge this and hopefully we can start using it right away!