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 description, validation_message, and introduced fields into openssl resources #6855

Merged
merged 1 commit into from
Feb 26, 2018

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Feb 14, 2018

Get these updated for the latest and greatest before we ship Chef 14

Signed-off-by: Tim Smith tsmith@chef.io

@tas50 tas50 requested a review from a team February 14, 2018 20:45
@tas50 tas50 force-pushed the openssl_fix branch 2 times, most recently from a438648 to 653cba8 Compare February 14, 2018 22:23
Copy link
Contributor

@thommay thommay left a comment

Choose a reason for hiding this comment

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

Lots of niggles on the english, but otherwise the change is good. On reflection I quite like the multiline property format.


property :private_key_content,
String,
description: "The content of the private key including new lines. Used if you don't want to write a private key to disk first then use `private_key_path`"
Copy link
Contributor

Choose a reason for hiding this comment

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

The second sentence has got a bit mangled here.

If you have a private key on disk already, use private_key_path

maybe?

property :owner, [String, nil]
property :group, [String, nil]
property :mode, [Integer, String], default: "0640"
description "A resource for generating rsa public key files given a rsa private key"
Copy link
Contributor

Choose a reason for hiding this comment

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

RSA is an acronym and should be capitalised across the board.

Integer,
equal_to: [1024, 2048, 4096, 8192],
validation_message: "key_length must be 1024, 2048, 4096, or 8192",
description: "The desired Bit Length of the generated key",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bit Length should not be capitalised.

property :mode, [Integer, String], default: "0600"
property :force, [true, false], default: false
introduced "14.0"
description "A resource for generating rsa private key files. If a valid rsa key"\
Copy link
Contributor

Choose a reason for hiding this comment

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

RSA should be capitalised.

Integer,
equal_to: [1024, 2048, 4096, 8192],
validation_message: "key_length must be 1024, 2048, 4096, or 8192",
description: "The desired Bit Length of the generated key",
Copy link
Contributor

Choose a reason for hiding this comment

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

Bit Length should not be capitalised.


property :private_key_path,
String,
description: "The path to the private key to generate the public key from"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just "The path to the private key" is fine here - the resource has already documented that it's generating a public key.


property :path,
String,
description: "The optional path to write the file to if you'd like to specify it here instead of in the resource name",
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to come up with less torturous wording for name properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

@tas50
Copy link
Contributor Author

tas50 commented Feb 17, 2018

I fixed up a few of them a bit

@tas50 tas50 force-pushed the openssl_fix branch 2 times, most recently from 02cefc8 to 99f7338 Compare February 21, 2018 20:06
…sl resources

This follows the pattern used on docs.chef.io right now. We may change
it later, but this will get us autogenerated docs for now.

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50 tas50 merged commit 7f12506 into master Feb 26, 2018
@tas50 tas50 deleted the openssl_fix branch February 26, 2018 20:21
@lock
Copy link

lock bot commented Apr 27, 2018

This thread has been automatically locked because it has not had recent activity. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked and limited conversation to collaborators Apr 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants