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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stivale2_struct_tag_boot_volume tag #11

Conversation

rdrpenguin04
Copy link
Contributor

Fixes #10

Here you go! This seemed like an important feature, especially for my hobby OS, which needs to basically fetch the entire rest of itself from the disk after two seconds; not knowing where that is would make it very difficult 馃檭.

This PR also pulls in the uuid crate. While this feature doesn't exactly require it to exist, it does provide an Into implementation for convenience.

@rdrpenguin04 rdrpenguin04 force-pushed the stivale2_struct_tag_boot_volume branch from 10dd755 to f030e31 Compare March 22, 2022 13:08
@rdrpenguin04
Copy link
Contributor Author

Force push was to disable uuid's default feature of using std

@rdrpenguin04 rdrpenguin04 force-pushed the stivale2_struct_tag_boot_volume branch from f030e31 to 5dd6f7a Compare March 22, 2022 13:19
@rdrpenguin04
Copy link
Contributor Author

Second force-push was to derive some standard traits on struct StivaleGuid.

Cargo.toml Outdated
@@ -12,3 +12,4 @@ categories = ["no-std"]

[dependencies]
bitflags = "1.3.2"
uuid = { version = "0.8.2", default-features = false }
Copy link
Owner

Choose a reason for hiding this comment

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

I don't really feel like pulling in this crate as "required" since in the whole crate its only used once and only for a .into() implementation. This can be made into a separate feature instead which if enabled will pull in the crate and implement the into conversion method. Or on the other hand, just make the user pull in the crate by themselves which is more neat IMO since you can construct the UUID crate struct with the stivale GUID easily outside of the crate. Then you can make the fields guid and part_guid private and make a method instead which returns a reference to the stivale GUID to ensure no one mutates the fields in the structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright; I'll make uuid optional. I still would like to have it, since it feels more convenient to be able to call .into() as opposed to writing out the whole constructor, but I'll bring it in as optional so people who don't need it don't get it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One more thing: it doesn't seem typical to make variables private, judging by the rest of the file; do you have a specific reason for this case?

@Andy-Python-Programmer Andy-Python-Programmer merged commit 42140ae into Andy-Python-Programmer:master Mar 23, 2022
@Andy-Python-Programmer
Copy link
Owner

Thanks @rdrpenguin04!

@rdrpenguin04
Copy link
Contributor Author

Uh, hang on a sec; the new changes don't work

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.

Missing stivale2_struct_tag_boot_volume
2 participants