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 minor device no and persist options #119

Conversation

robinbowes
Copy link
Contributor

I needed to create LVs with persistent block device IDs (for use a ext4 journal devices)

This change surfaces the --persistence and --minor options to the lvcreate command.

@robinbowes
Copy link
Contributor Author

newparam(:minor) do
desc "Set the minor number"
validate do |value|
unless value =~ /^(?:[0-9]|[1-9][0-9]|1[0-9][0-9]|2[0-4][0-9]|25[0-5])$/
Copy link
Contributor

Choose a reason for hiding this comment

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

How about if value.to_i > 255 or value.to_i < 0 instead of a regex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a fair cop, guv.

@bmjen
Copy link
Contributor

bmjen commented Jul 23, 2015

A couple notes:

  1. The new parameters need to be documented in the README and the examples
  2. The :persistent parameter definition should follow common idioms with a boolean value ( true or false), then the y or n values applied in the provider.

Changes following comments on original pull request
@robinbowes robinbowes force-pushed the feature/add_minor_device_no_and_persist_options branch from 5500ea6 to 49faba3 Compare July 25, 2015 21:31
@robinbowes
Copy link
Contributor Author

Updated to address @hunner's comment, and @bmjen's 2nd point.

Regarding the README, I'd be happy to add the new params and some examples to the README if it were in some sort of sane format that made that relatively easy. However, it seems to me that the README needs a complete overhaul to bring it into line with the recommended standard layout. I think that is out of the scope of this PR.

bmjen added a commit that referenced this pull request Sep 3, 2015
…sist_options

add minor device no and persist options
@bmjen bmjen merged commit 85fd4fa into puppetlabs:master Sep 3, 2015
@abednarik
Copy link

Thi issue should be closed rigth https://tickets.puppetlabs.com/browse/MODULES-2261?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants