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

Add documentation for CMS_EnvelopedData_create() #11529

Closed
wants to merge 1 commit into from

Conversation

bukka
Copy link
Contributor

@bukka bukka commented Apr 12, 2020

Documentation for CMS_EnvelopedData_create() functions.

Checklist
  • documentation is added or updated

@slontis
Copy link
Member

slontis commented Apr 14, 2020

I am not sure why missingcrypto.txt doesn't complain.

@bukka
Copy link
Contributor Author

bukka commented May 3, 2020

Is there anything else that I should do in this PR? It's basically the base for the last doc thing in the CMS AES-GCM PR #8024 that I need to do so if someone could take a look, that would be awesome. :)

@beldmit
Copy link
Member

beldmit commented May 3, 2020

LGTM.

@beldmit beldmit added approval: otc review pending This pull request needs review by an OTC member branch: master Merge to master branch labels May 3, 2020
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.

Like @slontis I'm not sure we're not seeing errors from make doc-nits complaining about this being in missingcrypto.txt. We should be...

Anyway. Please remove this function from util/missingcrypto.txt

doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
@bukka bukka force-pushed the cms-env-data-create-doc branch 2 times, most recently from f40f5e4 to 26a9ce1 Compare May 5, 2020 18:25
@bukka
Copy link
Contributor Author

bukka commented May 5, 2020

@mattcaswell all your comments hopefully addressed. Thanks for the review!

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.

Looks good apart from one last remaining nit below.

doc/man3/CMS_EnvelopedData_create.pod Outdated Show resolved Hide resolved
@bukka bukka force-pushed the cms-env-data-create-doc branch from 26a9ce1 to 241a67b Compare May 5, 2020 22:00
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.

LGTM.

@beldmit - please can you reconfirm?

@mattcaswell mattcaswell added approval: review pending This pull request needs review by a committer and removed approval: otc review pending This pull request needs review by an OTC member labels May 5, 2020
@beldmit beldmit 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 May 5, 2020
@mattcaswell
Copy link
Member

Travis red cross appears unrelated.

@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.

@beldmit
Copy link
Member

beldmit commented May 7, 2020

Merged. Thanks!

@beldmit beldmit closed this May 7, 2020
openssl-machine pushed a commit that referenced this pull request May 7, 2020
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from #11529)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants