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 more description fields, style fixes, add missing requires #6943

Merged
merged 3 commits into from Mar 5, 2018

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Mar 5, 2018

Make sure to require chef/resource everywhere. It would work without it but only because some other resource had already required it.

Put the type on the same line as the property, which we discussed in slack

Put introduced after description which makes differing simpler

Convert some yard into description fields

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

@tas50 tas50 requested a review from a team March 5, 2018 00:36
Make sure to require chef/resource everywhere. It would work without it but only because some other resource had already required it.

Put the type on the same line as the property, which we discussed in slack

Put introduced after description which makes differing simpler

Convert some yard into description fields

Signed-off-by: Tim Smith <tsmith@chef.io>
description "Use the build_essential resource to install packages required for compiling C software from source"
introduced "14.0"

property :compile_time, [true, false],

Choose a reason for hiding this comment

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

Shouldn't the type (and default) here be [TrueClass, FalseClass] for consistency?

description "Use the macos_userdefaults resource to manage the macOS user defaults"\
" system. The properties to the resource are passed to the defaults command"\
" and the parameters follow convention of the macOS command. See the defaults(1)"\
" man page for details on how the tool works."
introduced "14.0"

property :domain,
String,

Choose a reason for hiding this comment

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

Move the type up to the property line for consistency with other resources?

Hash,
description: "Values to include in the hint file"
property :content, Hash,
description: "Values to include in the hint file."

property :compile_time,
[TrueClass, FalseClass],

Choose a reason for hiding this comment

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

Move the type up to the property line for consistency with other resources?


default_action :install

allowed_actions :install, :remove

property :profile_name, String, name_property: true

Choose a reason for hiding this comment

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

Should name_property be on the same line as the property name? Looks inconsistent with other modernized resources.

description: "The username to use when registering. Not applicable if using an activation key. If specified, password and environment are also required."

property :password,
String,
property :password, String,
description: "The password to use when registering. Not applicable if using an activation key. If specified, username and environment are also required."

property :auto_attach,
[TrueClass, FalseClass],

Choose a reason for hiding this comment

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

Why not move this one up to the property line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't breaking them up until I added the descriptions. That bloats the lines so much it's easier to read on individual lines.

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50 tas50 merged commit e6b9ac4 into master Mar 5, 2018
@tas50 tas50 deleted the misc_resource branch March 5, 2018 18:16
@lock
Copy link

lock bot commented May 4, 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 as resolved and limited conversation to collaborators May 4, 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.

None yet

4 participants