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

Code formatting with clang-format #1963

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

remia
Copy link
Collaborator

@remia remia commented Apr 9, 2024

This adds a clang-format target, based on OpenImageIO implementation, to automatically format the source code with the .clang-format profile. Hoping to have feedback on the coding style to nail down any particular aspects that would currently be suboptimal. Here are a few notes:

  • In some places, clang-format makes things worse I added some clang-format off / on around places I found relevant to preserve. Includes the GPU shader text based generation, SSE related code (including SSE.h as a whole), multiple places where matrices or image array data readability were getting negatively impacted.
  • Unit tests are currently off the auto formatting as there would be too many places to manually protect for the time being.
  • Variable initialization using initializers list alignment tends to get lost, for exemple this (OpenColorTransforms.h line 461), I don't think clang-format can do any better at the moment.
  • Include order sorting is currently enabled and it broke compilation which I had to patch (for exemple when Platform.h gets included before OpenColorIO.h or gl.h before glew.h), can be disabled if we think it's safer.
  • clang-format version matters and different version produce different results, we should probably pin the version in the CI. For developer, this means using the same version which is made easier with the Python wheel provided for clang-format.

Signed-off-by: Rémi Achard <[email protected]>
Signed-off-by: Rémi Achard <[email protected]>
@remia remia marked this pull request as draft April 9, 2024 18:25
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

Successfully merging this pull request may close these issues.

1 participant