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

Remove dbus enums final #1222

Merged
merged 4 commits into from
Sep 26, 2018

Conversation

agrover
Copy link
Contributor

@agrover agrover commented Sep 25, 2018

final PR of #1204

fixes #1167

@agrover agrover added this to the Stratis 1.0 milestone Sep 25, 2018
@agrover agrover requested review from mulkieran and trgill September 25, 2018 21:47
@mulkieran
Copy link
Member

Whoops! You should fix up test infrastructure, too: tests/client-dbus/src/stratisd_client_dbus/_stratisd_constants.py. It looks like it was just the random luck of alphabetization that tests didn't fail. That is tests check for ALREADY_FOUND, BUSY, but they keep their numeric values after the change.

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.

See comment.

@agrover
Copy link
Contributor Author

agrover commented Sep 25, 2018

fixes #1160

@agrover
Copy link
Contributor Author

agrover commented Sep 25, 2018

I don't understand, this PR updates _stratisd_constants.py. What further needs to be changed?

mulkieran
mulkieran previously approved these changes Sep 26, 2018
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.

I must have looked at something unrelated by mistake. It looks good.

tasleson
tasleson previously approved these changes Sep 26, 2018
Andy Grover added 3 commits September 26, 2018 10:29
Best to just use ERROR.

fixes stratis-storage#1181

Signed-off-by: Andy Grover <agrover@redhat.com>
This is pretty much the same as an IO error.

Users should not care about which crates Stratis is using.

Signed-off-by: Andy Grover <agrover@redhat.com>
There's no bright-line between ERROR and IO_ERROR. Also, we should only
try to have separate error types for cases users will conceivably check
for and then do something, and IO_ERROR doesn't meet this test.

Signed-off-by: Andy Grover <agrover@redhat.com>
@agrover agrover dismissed stale reviews from tasleson and mulkieran via a1e9500 September 26, 2018 17:29
@agrover agrover force-pushed the remove-dbus-enums-final branch from 94f5fa4 to a1e9500 Compare September 26, 2018 17:29
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.

re-ack.

Copy link
Contributor

@tasleson tasleson left a comment

Choose a reason for hiding this comment

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

LGTM, again 😃

@mergify mergify bot merged commit 5498fe2 into stratis-storage:master Sep 26, 2018
@agrover agrover deleted the remove-dbus-enums-final branch September 26, 2018 20:51
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.

Remove bad DbusErrorEnum enums such as NIX_ERROR
4 participants