Skip to content

Comments

add clang-format as a pre-commit hook#29282

Closed
quarckster wants to merge 1 commit intoopenssl:masterfrom
quarckster:pre-commit-clang
Closed

add clang-format as a pre-commit hook#29282
quarckster wants to merge 1 commit intoopenssl:masterfrom
quarckster:pre-commit-clang

Conversation

@quarckster
Copy link
Contributor

@quarckster quarckster commented Dec 2, 2025 •
edited
Loading

This PR is a part of the effort for openssl reformat and adds config for pre-commit to run clang-format. It must be merged after #29242.

Closes openssl/project#1742

@quarckster quarckster requested a review from bob-beck December 2, 2025 08:00
@quarckster quarckster self-assigned this Dec 2, 2025
@quarckster quarckster moved this to In Progress in Development Board Dec 2, 2025
@quarckster quarckster moved this from In Progress to Blocked in Development Board Dec 2, 2025
@t8m t8m added branch: master Applies to master branch triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present branch: 3.0 Applies to openssl-3.0 branch branch: 3.3 Applies to openssl-3.3 branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 labels Dec 2, 2025
@loci-dev loci-dev mentioned this pull request Dec 2, 2025
Open
bob-beck
bob-beck reviewed Dec 3, 2025
View reviewed changes
@quarckster quarckster force-pushed the pre-commit-clang branch 2 times, most recently from d1c4b70 to 1aa6858 Compare December 4, 2025 07:49
@t8m
Copy link
Member

t8m commented Dec 4, 2025 •
edited
Loading

@quarckster just FYI - no need to submit backport PRs as the conflicts will be trivial to resolve when merging.

@quarckster
Copy link
Contributor Author

quarckster commented Dec 4, 2025 •
edited
Loading

@quarckster just FYI - no need to submit backport PRs as the conflicts will be trivial to resolve when merging.

Ack, thanks for letting me know.

@t8m
Copy link
Member

t8m commented Dec 4, 2025

@quarckster just FYI - no need to submit backport PRs as the conflicts will be trivial to resolve when merging.

Hmm... but actually we will need them for pre-commit CI addition for 3.5 and below. And to run the pre-commit ci on the reformatted tree to check that it passes it will be actually good to have a separate PR for each active branch. So I retract my suggestion.

@quarckster
Copy link
Contributor Author

quarckster commented Dec 4, 2025

@quarckster just FYI - no need to submit backport PRs as the conflicts will be trivial to resolve when merging.

Hmm... but actually we will need them for pre-commit CI addition for 3.5 and below. And to run the pre-commit ci on the reformatted tree to check that it passes it will be actually good to have a separate PR for each active branch. So I retract my suggestion.

no problem

@bob-beck
Copy link
Contributor

bob-beck commented Dec 4, 2025

@quarckster just FYI - no need to submit backport PRs as the conflicts will be trivial to resolve when merging.

Hmm... but actually we will need them for pre-commit CI addition for 3.5 and below. And to run the pre-commit ci on the reformatted tree to check that it passes it will be actually good to have a separate PR for each active branch. So I retract my suggestion.

no problem

indeed, and each of those will depend upon a different one of my formatting commits. I.e. we'll land the head and turn it on there before each of the branches, so may as well have the dependencies correct

@quarckster quarckster force-pushed the pre-commit-clang branch 4 times, most recently from 2f9147d to 17586e7 Compare December 9, 2025 11:55
t8m
t8m reviewed Dec 9, 2025
View reviewed changes
@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 9, 2025
@quarckster quarckster force-pushed the pre-commit-clang branch 4 times, most recently from 88fab7c to ccdd3b4 Compare December 9, 2025 13:03
@github-actions github-actions bot removed the severity: fips change The pull request changes FIPS provider sources label Dec 9, 2025
@quarckster quarckster marked this pull request as ready for review December 9, 2025 15:22
Sashan
Sashan approved these changes Dec 9, 2025
View reviewed changes
Copy link
Contributor

@Sashan Sashan left a comment

Choose a reason for hiding this comment

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

looks good thanks.

@github-project-automation github-project-automation bot moved this from Blocked to Waiting Merge in Development Board Dec 9, 2025
@Sashan Sashan added approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge labels Dec 9, 2025
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
d6f3733 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
635cbcc 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)

(cherry picked from commit d6f3733)
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
8622537 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)

(cherry picked from commit d6f3733)
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
6268be6 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)

(cherry picked from commit d6f3733)
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
f9c44f6 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)

(cherry picked from commit d6f3733)
@nhorman
Copy link
Contributor

nhorman commented Dec 9, 2025

merged to all active branches, thank you!

@nhorman nhorman closed this Dec 9, 2025
@github-project-automation github-project-automation bot moved this from Waiting Merge to Done in Development Board Dec 9, 2025
openssl-machine pushed a commit that referenced this pull request Dec 9, 2025
@quarckster @nhorman
add clang-format as a pre-commit hook
e0b0458 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from #29282)

(cherry picked from commit d6f3733)
cxx194832 pushed a commit to cxx194832/openssl that referenced this pull request Dec 12, 2025
@quarckster @cxx194832
add clang-format as a pre-commit hook
01a70d2 
Reviewed-by: Neil Horman <[email protected]>
Reviewed-by: Saša Nedvědický <[email protected]>
(Merged from openssl#29282)
This was referenced Jan 14, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t8m t8m t8m left review comments

@Sashan Sashan Sashan approved these changes

@nhorman nhorman nhorman approved these changes

+1 more reviewer

@bob-beck bob-beck bob-beck approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@quarckster quarckster

Labels

approval: done This pull request has the required number of approvals approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch branch: 3.3 Applies to openssl-3.3 branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug

Projects

Development Board
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Disable the legacy format checker in CI after clang-format

5 participants

@quarckster @t8m @bob-beck @nhorman @Sashan