Skip to content

Comments

make snmpkdf.c good openssl citizen#29369

Closed
Sashan wants to merge 2 commits intoopenssl:masterfrom
Sashan:29195.cstyle
Closed

make snmpkdf.c good openssl citizen#29369
Sashan wants to merge 2 commits intoopenssl:masterfrom
Sashan:29195.cstyle

Conversation

@Sashan
Copy link
Contributor

@Sashan Sashan commented Dec 10, 2025

cstyle change only

Checklist
  • documentation is added or updated
  • tests are added or updated

@Sashan
Copy link
Contributor Author

Sashan commented Dec 10, 2025 •
edited
Loading

this PR got created using this shell script equivalent:

for i in include/openssl/core_names.h.in include/openssl/proverr.h include/openssl/self_test.h providers/common/provider_err.c providers/defltprov.c providers/fips/fipsprov.c providers/implementations/include/prov/implementations.h providers/implementations/include/prov/names.h providers/implementations/kdfs/snmpkdf.c ; do
   clang-format-21 -i $i
done
for i in `git diff |grep '^+++' |sed -e 's/^+++ b\///g'` ; do
   git add $i;
done

so the only file needs clang format is indeed providers/implementations/kdfs/snmpkdf.c all others are fine.

(EDIT: fixed formatting of comment)

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Dec 10, 2025
@Sashan Sashan added branch: master Applies to master branch triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Dec 10, 2025
esyr
esyr previously approved these changes Dec 10, 2025
View reviewed changes
paulidale
paulidale previously approved these changes Dec 10, 2025
View reviewed changes
@paulidale paulidale added the approval: done This pull request has the required number of approvals label Dec 10, 2025
simo5
simo5 requested changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

please add providers/implementations/kdfs/snmpkdf.inc in .gitignore as part of this change

@simo5 simo5 mentioned this pull request Dec 10, 2025
Closed
2 tasks
@t8m t8m added the tests: exempted The PR is exempt from requirements for testing label Dec 11, 2025
@simo5 simo5 mentioned this pull request Dec 11, 2025
Closed
2 tasks
@beldmit beldmit removed the approval: done This pull request has the required number of approvals label Dec 11, 2025
@beldmit
Copy link
Member

beldmit commented Dec 11, 2025

I removed the "Approval:done" label, I agree with Simo's request
Please correct me if I'm wrong

@simo5
Copy link
Contributor

simo5 commented Dec 12, 2025

@Sashan any chance you can add the file in .gitignore everything looks good otherwise

@Sashan Sashan dismissed stale reviews from paulidale and esyr via 7ffe4d1 December 12, 2025 14:50
@Sashan
Copy link
Contributor Author

Sashan commented Dec 12, 2025

providers/implementations/kdfs/snmpkdf.inc

it's done. sorry I have not come back here earlier

@t8m t8m added the approval: review pending This pull request needs review by a committer label Dec 12, 2025
@t8m t8m requested review from esyr, paulidale and simo5 December 12, 2025 14:55
@simo5 simo5 added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Dec 12, 2025
@simo5
Copy link
Contributor

simo5 commented Dec 12, 2025

@t8m given the change requested was trivial I wonder if we could skip the 24h period

@openssl-machine
Copy link
Collaborator

openssl-machine commented Dec 13, 2025

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@beldmit beldmit removed the approval: done This pull request has the required number of approvals label Dec 13, 2025
@beldmit beldmit added the approval: ready to merge The 24 hour grace period has passed, ready to merge label Dec 13, 2025
@Sashan Sashan mentioned this pull request Dec 15, 2025
Closed
openssl-machine pushed a commit that referenced this pull request Dec 15, 2025
@Sashan
make snmpkdf.c good openssl citizen
42d4644 
cstyle change only

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from #29369)
@beldmit
Copy link
Member

beldmit commented Dec 15, 2025

Pushed. Thanks for your contribution!

@beldmit beldmit closed this Dec 15, 2025
openssl-machine pushed a commit that referenced this pull request Dec 15, 2025
@Sashan @beldmit
make snmpkdf.c good openssl citizen
0cf2595 
cstyle change only

Reviewed-by: Simo Sorce <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Dmitry Belyavskiy <[email protected]>
(Merged from #29369)
Sashan added a commit to Sashan/openssl that referenced this pull request Dec 16, 2025
@Sashan
make snmpkdf.c good openssl citizen
a117d17 
cstyle change only

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Simo Sorce <[email protected]>
(Merged from openssl#29369)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@t8m t8m t8m approved these changes

@simo5 simo5 simo5 approved these changes

@esyr esyr Awaiting requested review from esyr

@paulidale paulidale Awaiting requested review from paulidale

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Sashan @beldmit @simo5 @openssl-machine @esyr @t8m @paulidale