8000 Tidy up *dev status methods more (final) by mulkieran · Pull Request #417 · stratis-storage/devicemapper-rs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Tidy up *dev status methods more (final) #417

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Feb 4, 2019
Merged

Tidy up *dev status methods more (final) #417

merged 5 commits into from
Feb 4, 2019

Conversation

mulkieran
Copy link
Member
@mulkieran mulkieran commented Feb 4, 2019 •
edited
Loading

The objects of this PR are:

  • Better encapsulation.
  • To be better prepared for a possible future refactoring that eliminates the concept of distinct kinds of devices entirely.
  • To eliminate some needless boiler plate.
  • To get rid of the last risky assertion involved in reading a single status line.
  • To get rid of some obsoleted comments.
  • To parse one last thinpool status value, held metadata root.

This is a continuation of #387 and really resolves #221. It supercedes #410.

@mulkieran
Implement FromStr for status structs
f538ecb 
It is better to have the implementation associated with the structure
that it builds rather than buried in the larger method. Also, this method
of getting something from a str is the expected way.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Abstract checking number of fields into one method
b6bab46 
Eliminate the assertion and substitute an error return instead.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Omit some obsoleted comments
11e6c9a 
The method no longer panics if there is an error parsing the status value.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Also parse thinpool status value held metadata root
def352b 
It has been omitted until now, because it seemed unclear how to parse it.
But it seems clearer now. Also, get rid of an obsoleted comment.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran
Also abstract making an error if an unexpected value found
840a3c8 
This eliminates some unnecessary code duplication and fixes a
copy-paste bug.

Signed-off-by: mulhern <amulhern@redhat.com>
@mulkieran mulkieran self-assigned this Feb 4, 2019
@mulkieran mulkieran changed the title Master status final Tidy up *dev status methods more (final) Feb 4, 2019
@mulkieran mulkieran mentioned this pull request Feb 4, 2019
Closed
@mergify mergify bot merged commit ade582f into stratis-storage:master Feb 4, 2019
@mulkieran mulkieran deleted the master-status-final branch February 4, 2019 20:27
@mulkieran mulkieran mentioned this pull request Feb 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tasleson tasleson tasleson approved these changes

@trgill trgill trgill approved these changes

Assignees

@mulkieran mulkieran

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

expect! in status() methods will probably lead to embarassment eventually
3 participants
@mulkieran @tasleson @trgill
0