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

fips module header inclusion fine-tunning #15974

Closed
wants to merge 5 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Jul 1, 2021

This avoids a few nasty include files being part of the fips module sources.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 1, 2021
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Nice work!

@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed severity: fips change The pull request changes FIPS provider sources labels Jul 1, 2021
@t8m t8m removed the approval: done This pull request has the required number of approvals label Jul 1, 2021
@t8m
Copy link
Member Author

t8m commented Jul 1, 2021

Approval is a little bit premature. It will need more fine-tunnings to satisfy all the various CI configurations.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jul 1, 2021
@t8m
Copy link
Member Author

t8m commented Jul 1, 2021

@mattcaswell please re-approve. It seems just this simple fixup was needed.

@t8m t8m added the approval: review pending This pull request needs review by a committer label Jul 1, 2021
Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

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

Nice. A good pile of headers that we don't want in FIPS removed.

@paulidale paulidale 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 Jul 1, 2021
@t8m
Copy link
Member Author

t8m commented Jul 2, 2021

A few more tweaks. Please re-approve. Unfortunately getting rid of include/openssl/asn1* and include/openssl/bio* seems too hard and invasive.

@openssl-machine
Copy link
Collaborator

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.

@mattcaswell mattcaswell added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Jul 5, 2021
@paulidale
Copy link
Contributor

Merged.

@paulidale paulidale closed this Jul 6, 2021
openssl-machine pushed a commit that referenced this pull request Jul 6, 2021
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15974)
openssl-machine pushed a commit that referenced this pull request Jul 6, 2021
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15974)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15974)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15974)
@t8m t8m deleted the fips-module-cuts branch May 11, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge severity: fips change The pull request changes FIPS provider sources
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants