Skip to content

Comments

Fix links in cms_add0_cert.pod and improve find-doc-nits error msg#20369

Closed
DDvO wants to merge 2 commits intoopenssl:masterfrom
siemens:fix_CMS_add0_cert.pod_and_error_msg
Closed

Fix links in cms_add0_cert.pod and improve find-doc-nits error msg#20369
DDvO wants to merge 2 commits intoopenssl:masterfrom
siemens:fix_CMS_add0_cert.pod_and_error_msg

Conversation

@DDvO
Copy link
Contributor

@DDvO DDvO commented Feb 24, 2023 •
edited
Loading

It turns out that my recently merged changes to cms_add0_cert.pod made find-doc-nits unhappy.
Sorry that this went unnoticed and now causes unrelated CI failures in newly pushed PRs.
For this reason, I propose making the fix given here urgent.

As the error messages by make doc-nits:

/usr/bin/perl ./util/find-doc-nits -c -n -l -e
doc/man3/CMS_add0_cert.pod:1: Section missing in CMS_sign_ex()
doc/man3/CMS_add0_cert.pod:1: Section missing in CMS_sign()
doc/man3/CMS_add0_cert.pod:1: Section missing in CMS_verify()
doc/man3/CMS_add0_cert.pod:1: Section missing in CMS_verify()

are unclear and even misleading, such that it took me a while to understand what went wrong,
I propose the improvement of find-doc-nits given in the 2nd commit.
The error given will then read, e.g.,:

doc/man3/CMS_add0_cert.pod:1: Missing man section number (likely, 3) in L<CMS_verify()>

@DDvO DDvO added approval: review pending This pull request needs review by a committer approval: otc review pending triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors) labels Feb 24, 2023
@DDvO DDvO mentioned this pull request Feb 24, 2023
Closed
2 tasks
@DDvO DDvO requested a review from t8m February 24, 2023 13:16
@mattcaswell mattcaswell added the severity: urgent Fixes an urgent issue (exempt from 24h grace period) label Feb 24, 2023
@mattcaswell
Copy link
Member

mattcaswell commented Feb 24, 2023

Agree urgent

@mattcaswell mattcaswell added the branch: master Applies to master branch label Feb 24, 2023
hlandau
hlandau approved these changes Feb 24, 2023
View reviewed changes
Copy link
Member

@hlandau hlandau left a comment

Choose a reason for hiding this comment

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

Agree urgent

@hlandau hlandau 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 approval: otc review pending labels Feb 24, 2023
openssl-machine pushed a commit that referenced this pull request Feb 24, 2023
@DDvO
CMS_add0_cert.pod: add missing man section numbers in recently added …
e6657e5 
…L<fun()> refs

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #20369)
openssl-machine pushed a commit that referenced this pull request Feb 24, 2023
@DDvO
util/find-doc-nits: improve error diagnostics on missing man section …
9a2f78e 
…numbers in links

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #20369)
@DDvO DDvO added branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) labels Feb 24, 2023
@DDvO
Copy link
Contributor Author

DDvO commented Feb 24, 2023 •
edited
Loading

Merged to master - thanks @mattcaswell and @hlandau for the swift approvals.

I just realized that the same happend also for 3.0 and 3.1 by the backport PR #20275.
Can you please approve for 3.0 and 3.1 as well?

@DDvO DDvO added approval: review pending This pull request needs review by a committer approval: otc review pending and removed approval: done This pull request has the required number of approvals labels Feb 24, 2023
@DDvO
Copy link
Contributor Author

DDvO commented Feb 24, 2023

Minor CI failures unfortunately are easily overlooked these days
due to the frequent CI false positives on certain Windows builds.

mattcaswell
mattcaswell approved these changes Feb 24, 2023
View reviewed changes
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.

Approved for 3.1/3.0 too

hlandau
hlandau approved these changes Feb 24, 2023
View reviewed changes
Copy link
Member

@hlandau hlandau left a comment

Choose a reason for hiding this comment

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

Approved as urgent for 3.0/3.1 as well

@hlandau hlandau 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 Feb 24, 2023
DDvO added a commit to siemens/openssl that referenced this pull request Feb 24, 2023
@DDvO
CMS_add0_cert.pod: add missing man section numbers in recently added …
becdf1f 
…L<fun()> refs

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#20369)

(cherry picked from commit e6657e5)
DDvO added a commit to siemens/openssl that referenced this pull request Feb 24, 2023
@DDvO
util/find-doc-nits: improve error diagnostics on missing man section …
522d624 
…numbers in links

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#20369)

(cherry picked from commit 9a2f78e)
DDvO added a commit to siemens/openssl that referenced this pull request Feb 24, 2023
@DDvO
CMS_add0_cert.pod: add missing man section numbers in recently added …
8e42ff1 
…L<fun()> refs

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#20369)

(cherry picked from commit e6657e5)
(cherry picked from commit becdf1f)
DDvO added a commit to siemens/openssl that referenced this pull request Feb 24, 2023
@DDvO
util/find-doc-nits: improve error diagnostics on missing man section …
e20e496 
…numbers in links

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from openssl#20369)

(cherry picked from commit 9a2f78e)
(cherry picked from commit 522d624)
@DDvO
Copy link
Contributor Author

DDvO commented Feb 24, 2023

Cherry-picked to 3.0 and 3.1. Thanks.

@DDvO DDvO closed this Feb 24, 2023
@t8m t8m reopened this Feb 28, 2023
@t8m t8m mentioned this pull request Feb 28, 2023
Closed
2 tasks
@openssl-machine
Copy link
Collaborator

openssl-machine commented Feb 28, 2023

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.

@paulidale
Copy link
Contributor

paulidale commented Feb 28, 2023 •
edited
Loading

If we cherry pick to 3.0 and 3.1 from here, it should only be commit c34be62 IMO.
For some reason the earlier pick didn't stick.

@DDvO
Copy link
Contributor Author

DDvO commented Feb 28, 2023

For some reason the earlier pick didn't stick.

Strange. Thanks for taking care of re-applying the doc fix in #20398.
So we can close this earlier PR again.

@DDvO DDvO closed this Feb 28, 2023
openssl-machine pushed a commit that referenced this pull request Mar 14, 2023
@DDvO
util/find-doc-nits: improve error diagnostics on missing man section …
2dde9fb 
…numbers in links

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
Reviewed-by: David von Oheimb <[email protected]>
(Merged from #20369)

(cherry picked from commit 9a2f78e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattcaswell mattcaswell mattcaswell approved these changes

@t8m t8m Awaiting requested review from t8m

+1 more reviewer

@hlandau hlandau hlandau approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approval: done This pull request has the required number of approvals branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch branch: 3.1 Applies to openssl-3.1 (EOL) severity: urgent Fixes an urgent issue (exempt from 24h grace period) triaged: bug The issue/pr is/fixes a bug triaged: documentation The issue/pr deals with documentation (errors)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@DDvO @mattcaswell @openssl-machine @paulidale @hlandau @t8m