-
Notifications
You must be signed in to change notification settings - Fork 581
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
Adjust incosistent pki log messages #8965
Conversation
02561a2
to
df8de02
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Write a test protocol.
Before: After:
|
And the other case? |
Which other case? |
|
Well, you've introduced an if with an else branch – two cases. One's already covered by #8965 (comment) – one not. |
|
Actually, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intention was not to give suggestions for the individual cases. What I wanted to say was to not attempt using different wording depending on the error cause but instead remove that if
altogether and use a single message in all cases.
I just didn't want to give exact wording. You also have to ensure that it fits the rest of the message, for example "Received certificate request for CN 'foo' verification failed: [...]" sounds broken.
c6edf2d
to
de33f5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the message is "Received certificate request for CN 'something' couldn't be verified: some error message", it should rather be "which couldn't be verified" to be correct.
de33f5b
to
a8db3f0
Compare
Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA). Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA. After that, please reply here with a comment and we'll verify. Contributors that have not signed yet: @yhabteab
|
Actually, I don't see any difference, but I have changed it the way you requested. |
On second thought, both versions are fine. Without "which", you have the read "received" as an adjective and not as the verb of the sentence (which I didn't in the first place). |
@cla-bot check |
Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA). Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA. After that, please reply here with a comment and we'll verify. Contributors that have not signed yet: @yhabteab
|
What's the problem here? |
The account was renamed and this change is probably not yet reflected in our CLA check. |
@cla-bot check |
Checked all the error codes and couldn't find any others that could be raised,
even though the crt was issued by CA, apart from the revoked CRTs.
resolves #8682