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

Adds scripts, Dockerfiles, and GitHub Actions CI for checking formatting and autoformatting #124

Merged
merged 11 commits into from
Dec 17, 2024

Conversation

ilumsden
Copy link
Collaborator

Since I know there's been some issues with formatting (mostly due to my VSCode being stupid), I put together this PR to provide some new tools to help us with code formatting. This PR is heavily based on LLNL/Caliper#605, which I put together for the same reason.

This PR adds several scripts to the repo (all can be found in scripts/formatting/):

  • check-formatting.sh: uses clang-format to check if the files in src/ and include/ are formatted correctly
  • autoformat.sh: uses clang-format to autoformat the files in src/ and include/
  • Dockerfile.check: a Dockerfile that runs check-formatting.sh with clang-format v17.0.6
  • Dockerfile.format: a Dockerfile that runs autoformat.sh with clang-format v17.0.6
  • check-formatting-docker.sh: a script that builds and runs Dockerfile.check
  • autoformat-docker.sh: a script that builds and runs Dockerfile.format

Additionally, this PR adds a new GitHub Actions workflow to check if code is formatted correctly. This script is essentially just a GitHub CI version of Dockerfile.check.

Finally, this PR adds the results of autoformatting the repo.

@ilumsden
Copy link
Collaborator Author

ilumsden commented Oct 17, 2024

  • Add newline to end of each file
  • Enforce ordering of includes
  • Tweak formatting to preserve include groups and sort within each group
  • Update docs to state that includes should be placed in newline-separated groups, which external groups (e.g., for C++ Standard Library and external tools) coming before internal groups
  • Merge Dockerfiles into hierarchical Dockerfile (note: figure out how to run a specific tag for this type of Dockerfile)

Copy link
Collaborator

@hariharan-devarajan hariharan-devarajan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets address the comments we made during meeting.

@ilumsden
Copy link
Collaborator Author

@hariharan-devarajan all the requested changes to this PR are complete.

src/dyad/common/dyad_rc.h Outdated Show resolved Hide resolved
@wangvsa wangvsa merged commit ceb0f6f into flux-framework:main Dec 17, 2024
4 checks passed
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.

3 participants