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

Make bus types enum constants instead of strings #106

Closed
jaypipes opened this issue Jan 27, 2019 · 6 comments
Closed

Make bus types enum constants instead of strings #106

jaypipes opened this issue Jan 27, 2019 · 6 comments
Milestone

Comments

@jaypipes
Copy link
Owner

The bus types are strings with a mix of uppercase and lowercase runes. Make them constant enums instead.

@jaypipes jaypipes added this to the 0.3 milestone Jan 27, 2019
jaypipes added a commit that referenced this issue Jan 29, 2019
Modifies the `ghw.Disk` struct's `BusType` attribute to be of type
`ghw.BusType` instead of `string`. Creates an "enum" of bus types,
including a new PCI bus type which will be useful in future patches for
auxiliary devices.

Issue #106
@jaypipes
Copy link
Owner Author

jaypipes commented Jan 29, 2019

@jpo-joyent @yrobla @pwFoo @blp1526 would you all mind doing a quick review of this patch please?

#109

If you are using ghw as a library (versus the ghwc CLI tool) and are referencing the ghw.Disk.BusType field, this patch will end up breaking your code if you haven't pinned your ghw dependency to 0.2 release or prior. Can you let me know if you're OK with this? ghw is still in a fluid pre-1.0 release state and I'm trying to clean things like this up before the 1.0 release.

Thank you!
-jay

@yrobla
Copy link

yrobla commented Jan 30, 2019

Change is ok for me . We are not yet using library (although we want to) , but using ghwc CLI tool.

@blp1526
Copy link
Contributor

blp1526 commented Jan 30, 2019

LGTM.

(This Makefile has make lint, and it warns us "don't use ALL_CAPS in Go names; use CamelCase (golint)" at bus_type.go. If ghw keeps using gometalinter, I think that it is better to modify the warning lines.)

@jaypipes
Copy link
Owner Author

Yeah, I'm not a fan of that particular Go-ism. Maybe it's my C background but I just like having constants use ALL_CAPS. So, that's not going to change... I might just remove the warning lines for that.

@jpo-joyent
Copy link
Contributor

Yep, broke my code but was trivial to fix. Thank you for giving us a heads up! :)

@jpo-joyent
Copy link
Contributor

Actually no, this breaks macOS again, and in a way that I can not easily fix.

In macOS, the bus type is a string that comes from the IOKit BusProtocol field, whose legal values are not documented, and whose source is not available. The only value I've seen is PCI-Express, but this does not mean other things are not possible, just that I have not seen them so far. BusType being a string let me just pass them through and let the caller have something useful in cases I have not anticipated, but now I don't have a clean option. Always pass BUS_TYPE_UNKNOWN? That seems less than ideal...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants