Skip to content

Conversation

movitto
Copy link
Contributor

@movitto movitto commented Sep 4, 2013

...space

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling c58cb83 on movitto:create_partitions_percentages into 07a33b8 on ManageIQ:master.

Copy link
Member

Choose a reason for hiding this comment

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

This is called via parted_options_array for print, mklabel and mkpart but I believe -a opt is only valid for mkpart. I think the others will fail. This should be only done for mkpart l believe.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f8f2902 on movitto:create_partitions_percentages into 07a33b8 on ManageIQ:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f8f2902 on movitto:create_partitions_percentages into 07a33b8 on ManageIQ:master.

@movitto
Copy link
Contributor Author

movitto commented Sep 5, 2013

@jrafanie updated

Copy link
Member

Choose a reason for hiding this comment

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

spacing is funny here and on the next few lines...looks like it needs an extra space.

@coveralls
Copy link

Coverage Status

Coverage decreased (-24.65%) when pulling 431b95d on movitto:create_partitions_percentages into 07a33b8 on ManageIQ:master.

@movitto
Copy link
Contributor Author

movitto commented Sep 5, 2013

@Fryguy updated

Copy link
Member

Choose a reason for hiding this comment

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

I just checked and Range#overlaps? and Range#include? are already provided by active_support and should be preferred over rolling our own.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling 8b4cf01 on movitto:create_partitions_percentages into 11a950a on ManageIQ:master.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like you can get rid of the partition =

@movitto
Copy link
Contributor Author

movitto commented Sep 9, 2013

@brandondunne @Fryguy updated

Lines split up to keep them < 80 chars long. Relaxed that rule in the spec suite, but in the main lib would rather keep to that max-width as it's pretty standard

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the context should be around creating multiple partitions context "multiple partitions" do, then have it "with valid params" do and it "with overlapping params" do, what do you think?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) when pulling b62ecdb on movitto:create_partitions_percentages into 11a950a on ManageIQ:master.

@movitto
Copy link
Contributor Author

movitto commented Sep 9, 2013

@brandondunne updated.

bdunne added a commit that referenced this pull request Sep 10, 2013
further parameterize disk#create_partition, accept params for % of free ...
@bdunne bdunne merged commit 789b2bd into ManageIQ:master Sep 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants