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

Add Additional MediaType and DeviceType variants; upgrade nix and strum #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iamjpotts
Copy link

@iamjpotts iamjpotts commented Jan 9, 2024

  • Vendor can now Display
  • Rename Vendor:Vbox to Vendor::VirtualBox
  • Rename Vendor::None to Vendor::ATA to match its vendor string
  • Make Device::vendor an Option that defaults to Option::None instead of Vendor::None when vendor is unknown
  • Add MediaType::Optical for when a CD or DVD drive (or a virtual variant of the same) is detected
  • Add MediaType::Partitions to represent detection of a partition table on a device
  • Add MediaType::Partition to represent when a device represents a partition
  • Rename MediaType::Virtual to MediaType::Qemu to distinguish it from Virtual Box media
  • Add MediaType::VirtualBox
  • Add MediaType::Unrecognised with a field to expose the vendor of unrecognized media
  • MediaType can now Display
  • Add DeviceType::Unrecognised to expose the value of a device type that is not represented as an enum variant

MediaType::Unknown still exists but is now only used when the matching rules don't have any input to work with.

  • Upgrade nix from 0.23 to 0.26. There is a newer version 0.27 but it has breaking changes.
  • Upgrade strum from 0.24 to 0.25
  • Update the cargo edition from 2018 to 2021

udev cannot be easily upgraded; there is a breaking change to Enumerator ownership related scan_devices which will require a significant refactor to some of the _iter functions.

Lastly I bumped the crate version from 0.11 to 0.12.0 to represent that this is a breaking change. [edited this comment to reflect your feedback]

Copy link
Owner

@cholcombe973 cholcombe973 left a comment

Choose a reason for hiding this comment

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

Great work. Yeah since this is a breaking change I usually would just bump the version to 0.12. I don't know if the alpha tag in addition is necessary.
You can bump the version along with this PR. I'm not picky 🙂.
I suppose my other thought with the alpha tag is how long would you want to leave it in place before promoting it to stable?

@iamjpotts
Copy link
Author

I've squished the two commits into one and changed the version to 0.12.0

In my own repos I often have a pre-release suffix on the version while there are unpublished changes in the default branch, but everyone's processes can be different.

@iamjpotts
Copy link
Author

@cholcombe973

Doing a bit more experimentation on more hardware and VMs. I think i'm going to make a revision to the PR and can improve the media type matching rule.

In the mean time, it would be helpful for CI to be enabled. It's disabled due to this being my first PR to this repo.

@cholcombe973
Copy link
Owner

Sure no problem. I thought I enabled it already but I clicked it again.

@iamjpotts
Copy link
Author

@cholcombe973 i've made my updates to the matching rules and enum variants. They're not perfect but they should be an improvement.

Tested on VirtualBox, a server with a raid controller, and a consumer desktop.

…ve MediaType detection. Upgrade some dependencies.
@iamjpotts
Copy link
Author

Added FilesystemType::Luks and deduplicated FilesystemType::to_str / impl fmt::Display for FilesystemType

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.

2 participants