-
Notifications
You must be signed in to change notification settings - Fork 856
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
BSD: find_devs_with_ refactoring #298
BSD: find_devs_with_ refactoring #298
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a fan of ad-hoc parsers, but this one was already in place, so, 🤷♀️
c8446d7
to
df56859
Compare
# Query optical drive to get it in blkid cache for 2.6 kernels | ||
util.find_devs_with(path="/dev/sr0") | ||
util.find_devs_with(path="/dev/sr1") | ||
# Query optical drive to get it in blkid cache for 2.6 kernels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still something useful?
df56859
to
544f26e
Compare
544f26e
to
76e71f0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, thanks for the refactor @goneri. Minor nits mentioned inline.
76e71f0
to
c35884e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks just leaving a couple of nits in there to see if you find them objectionable and then we can land either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 @goneri just a git rebase -i master and resolve a minor conflict now that your other branch landed.
Refactoring of the `find_devs_with_xxx()` methods: - centralize everything in `util.py` - add test coverage
b9ceace
to
0f4a189
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great thanks a ton.
Refactoring of the
find_devs_with_xxx()
methods:util.py