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.

Sign up for GitHub

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

Jump to bottom

rename public DiskExt method type_ to kind #966

Merged
merged 2 commits into from Apr 3, 2023
Merged

rename public DiskExt method type_ to kind #966

merged 2 commits into from Apr 3, 2023

Conversation

jRimbault
Copy link
Contributor

@jRimbault jRimbault commented Apr 3, 2023 •
edited

A cosmetic change, but a breaking one also.

The underscore at the end of type_ has bothered me for some time now. Maybe using a proximate synonym could be better?

@jRimbault
rename public DiskExt method type_ to kind
a27a062 
a cosmetic change, but a breaking one also
GuillaumeGomez
GuillaumeGomez reviewed Apr 3, 2023
View reviewed changes
src/common.rs Outdated
///
/// This type is returned by [`Disk::get_type`][crate::Disk#method.type].
/// This type is returned by [`crate::DiskExt::kind`].
Copy link
Owner

Choose a reason for hiding this comment

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

It'll be displayed as crate::DiskExt::kind, which isn't great. Can you use the old format please?

@GuillaumeGomez
Copy link
Owner

I don't mind. Please fix the CI errors and then all good for me.

@jRimbault
Copy link
Contributor Author

jRimbault commented Apr 3, 2023 •
edited

The CI seems stuck on the same errors encountered on the other branches, check_cpu_usage and wasm-pack.

@GuillaumeGomez
Copy link
Owner

Thanks!

@GuillaumeGomez GuillaumeGomez merged commit ebb01c4 into GuillaumeGomez:master Apr 3, 2023
62 of 68 checks passed
@jRimbault
Copy link
Contributor Author

In true esprit d'escalier fashion. It should probably be all renamed to StorageKind and StorageExt. Since SSDs and Unknown are not disks.

@GuillaumeGomez
Copy link
Owner

That's a good point. Maybe even StorageDrive or StorageDevice? I'm not sure which one is better to be as easy for users to find as possible.

@jRimbault
Copy link
Contributor Author

I think "Drive" is (except in the HDD acronym) mostly Windows parlance for a partition mounted on a root letter? I've currently got on Windows several "drives" which are in fact the same device. "Device" seems more generic to me.

@GuillaumeGomez
Copy link
Owner

But Device could be pretty much anything, hence why I thought "StorageDevice" would be a better idea.

@jRimbault
Copy link
Contributor Author

That's what I was trying to say, "storage device" seems better here

@GuillaumeGomez
Copy link
Owner

Oh sorry. Then yes, we agree. :)

@GuillaumeGomez GuillaumeGomez mentioned this pull request May 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jRimbault @GuillaumeGomez