search

electro-smith / libDaisy

Hardware Library for the Daisy Audio Platform
https://www.electro-smith.com/daisy
MIT License
312 stars 131 forks source link

Improvements to clang-format project integration #573

Open stephenhensley opened 1 year ago

stephenhensley commented 1 year ago

@TheSlowGrowth had a few great ideas in #566 and before some time goes by and they're lost to time, I figured I'd move them here:

In most commerical repos I worked on in the past, we had a local clang-format binary commited to a tools/ subdirectory in the repo - to avoid these issues alltogether. VSCode has the benefit that we can specify a local clang-format executable in the repository's .vscode/settings.json file so that this local version is automatically used. We could take this even further and install a git hook that, when you make a commit, automatically warns about wrong formatting, even before the commit is pushed to origin.

. . . they have to be installed once after initially cloning the repository. We could help users by providing a tasks.json entry for VScode, so that all they have to do is to run this task once. My suggestion would be to use something like the pre-commit tool to handle all of this. The benefit of this tool is that it automatically downloads the correct clang-format binary. It can also be used on the client side as well as the server side (which we still need because the client side hooks are not enforce-able). The only problem of this tool is that it requires python, which is not included in the toolchain installer, as far as I can tell. An alternative solution would be bash scripts and clang-format binaries commited to the repo. Maybe that would work better for us.

The workflow would be:

  • developer makes a commit in their local working copy.
  • if ill-formatted: commit is rejected by the pre-commit hook and the suggested changes are written to the working copy. User can just stage the changes and commit again. This time, the commit is accepted and can be pushed to origin.

This would solve two problems:

  1. Differences between different clang-format executables not lining up with the same version used by CI
  2. Reducing initial back-and-forth at the start of many PRs due to clang-format styling having been done, but perhaps not matching the expectation checked by CI.