8000 Parse value final by agrover · Pull Request #387 · stratis-storage/devicemapper-rs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Parse value final #387

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 4 commits into from
Nov 16, 2018
Merged

Parse value final #387

merged 4 commits into from
Nov 16, 2018

Conversation

agrover
Copy link
Contributor
@agrover agrover commented Nov 16, 2018 •
edited by mulkieran
Loading

Final PR based on work PR #384.

Resolves: #221.

Andy Grover added 4 commits November 15, 2018 16:29
Add parse_value()
2bc93ee 
Unify parsing of numbers into values with a function. Since we want to
say WHICH number parsing was bad, also take a description of what was being
parsed.

fixes stratis-storage#221

Signed-off-by: Andy Grover <agrover@redhat.com>
Replace some panic()s with error returns
942556a 
Since these functions already return a DmResult, there really isn't any
return-value simplification to be had by panicing vs returning yet another
error.

Signed-off-by: Andy Grover <agrover@redhat.com>
Change parse_pairs() to return a DmResult
f9bbb48 
In comparison to the previous commit, here we do need to change the return
value from parse_pairs(). But, since callers of parse_pairs() already
returned a DmResult, callsites hardly need to change.

Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: Andy Grover <agrover@redhat.com>
Change parse_device() to take a description
5415120 
This lets the error message include info for which value failed to parse.

Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: Andy Grover <agrover@redhat.com>
@agrover agrover added this to the Stratis 1.1 milestone Nov 16, 2018
@agrover agrover mentioned this pull request Nov 16, 2018
Closed
mulkieran
mulkieran approved these changes Nov 16, 2018
View reviewed changes
Copy link
Member
@mulkieran mulkieran left a comment

Choose a reason for hiding this comment

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

B972

This resolves #221, so I added that in the PR message. AFAIAC, it can now go in at any time.

This was referenced Nov 16, 2018
Closed
Closed
@mergify mergify bot merged commit c1655bc into stratis-storage:master Nov 16, 2018
This was referenced Jan 29, 2019
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mulkieran mulkieran mulkieran approved these changes

@trgill trgill trgill approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Stratis 1.1
Development

Successfully merging this pull request may close these issues.
3 participants
@agrover @mulkieran @trgill
0