Fix getting properties #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Use syspath instead of sysname for udev - Add dev_path_to_sys_path() function - Return error if there no sys path
|
I've found some potential problems, please don't merge now. |
|
Doesn't matter, it were unrelated problems with Fedora 33. |
|
This PR looks reasonable to me. Would you like me to merge it? |
|
@cholcombe973 Yes, thank you :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #22
There was a bug with incorrect
syspathforudev. I've fixed it making an assumption we work only with block devices (and return theErrorotherwise) and thesyspathis always like/sys/class/block/.I've tested it locally and with the integration tests in our project where we use
block-utils. However, it's a bit complicated to write unit tests in the scope of this repo because it requires a kind of testing framework for loop devices, disks, partitions, filesystems, and so on. We have a kind of such one in progress, but it hasn't opensourced yet.Well, I'm not sure by 100% that the assumption about
/sys/class/blockis general truth and this path exists for all block devices. There are a lot of other sys paths like/sys/devices/virtual/block/and/sys/block/. Anyway, it looks like/sys/class/blockis an expectedsyspathaccording to the examples inudev.