Skip to content

Run clang-format inside a pinned docker image#3472

Open
iyiola-dev wants to merge 2 commits into
open-telemetry:mainfrom
iyiola-dev:clang-format-docker
Open

Run clang-format inside a pinned docker image#3472
iyiola-dev wants to merge 2 commits into
open-telemetry:mainfrom
iyiola-dev:clang-format-docker

Conversation

@iyiola-dev
Copy link
Copy Markdown

Update the Makefile clang-format target to invoke clang-format inside a Docker image pinned in dependencies.Dockerfile
Resolves #2774

@iyiola-dev iyiola-dev requested a review from a team as a code owner April 27, 2026 13:43
Update the Makefile clang-format target to invoke clang-format inside a Docker image pinned in dependencies.Dockerfile so the tool runs at a consistent version (matching the clang-format-19 used in CI) for every contributor.

Resolves open-telemetry#2774
@iyiola-dev iyiola-dev force-pushed the clang-format-docker branch from c4dcebc to b81d1a2 Compare April 27, 2026 13:47
Copy link
Copy Markdown
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This looks like a useful improvement. Moving make clang-format to a pinned Docker image makes that workflow more reproducible and removes one source of local version drift.

One thing I’d want to call out is scope: this change standardizes the make clang-format path, but it doesn’t yet bring the other formatter entry points onto the same toolchain. The CI check still installs and runs clang-format-19 directly, and the pre-commit hook still calls the host clang-format, so we can still get different behavior depending on how formatting is run.

I don’t think that blocks this PR if the goal here is specifically to fix the Makefile workflow. But I do think it would help to leave a note about expected follow-up, so readers don’t assume this change fully unifies formatting across local development and CI.

Extend the dockerized clang-format setup to the clang-format-check CI workflow and the local pre-commit hook so all three entry points use the image pinned in dependencies.Dockerfile.
@iyiola-dev
Copy link
Copy Markdown
Author

This looks like a useful improvement. Moving make clang-format to a pinned Docker image makes that workflow more reproducible and removes one source of local version drift.

One thing I’d want to call out is scope: this change standardizes the make clang-format path, but it doesn’t yet bring the other formatter entry points onto the same toolchain. The CI check still installs and runs clang-format-19 directly, and the pre-commit hook still calls the host clang-format, so we can still get different behavior depending on how formatting is run.

I don’t think that blocks this PR if the goal here is specifically to fix the Makefile workflow. But I do think it would help to leave a note about expected follow-up, so readers don’t assume this change fully unifies formatting across local development and CI.

Hi, will extend the scope of this to the ci and the pre-commit hook

@iyiola-dev
Copy link
Copy Markdown
Author

hi @MrAlias extended the PR to dockerize the remaining two entry points so all three (make clang-format, the clang-format-check workflow, and hooks/pre-commit) now run from the same image pinned in dependencies.Dockerfile:

  • .github/workflows/clang-format-check.yml drops the apt-get install clang-format-19 step and calls docker run against the pinned image. The image reference is read from dependencies.Dockerfile via the same awk lookup the Makefile uses, so Renovate updates all three entry points from the single image declaration.
  • hooks/pre-commit resolves the repo root via git rev-parse --show-toplevel (so it works from subdirectories) and formats all staged files in a single docker run invocation.

Updated the CHANGELOG entry to reflect the broader scope. Verified locally:

  • The docker run ... clang-format --version call returns 19.1.7, matching the clang-format-19 previously installed by the workflow.
  • The pre-commit hook correctly picks up staged .c/.h files, runs clang-format inside the container, and re-stages the formatted files.

@iyiola-dev
Copy link
Copy Markdown
Author

hi @MrAlias do you have any review here yet for this?

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.

Run clang-format in a docker image

2 participants