-
Notifications
You must be signed in to change notification settings - Fork 178
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
doc and const cleanup for cpu, block, memory, and net packages #352
Conversation
Ensures proper docstrings for all exported things in the `memory` package and deprecates the (non-Go-idiomatic) ALL_CAPS constants in favour of CamelCase constants. Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Ensures that we sort the `ProcessorCore.LogicalProcessors` slice of ints for each processor core in Linux. Also update the documentation for the CPU package to mark the `Processor.Cores` and `Processor.Capabilities` fields as Linux-only. Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Ensures proper docstrings for all exported things in the `block` package and deprecates the (non-Go-idiomatic) ALL_CAPS constants in favour of CamelCase constants. Also officially deprecates the `pkg/block.Info.TotalPhysicalBytes` field in favour of a `pkg/block.Info.TotalSizeBytes` field (had a TODO in there a while to do this...). Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Ensures proper docstrings for all exported things in the `net` package and deprecates the (non-Go-idiomatic) ALL_CAPS constants in favour of CamelCase constants. Signed-off-by: Jay Pipes <jaypipes@gmail.com>
@ffromani mostly docstring updates in here. sorry for the large PR. Feel free to review each commit separately if that's easier for you. |
@ffromani heya :) any chance to do a quick review on this one today? |
yes, sorry. I'll squeeze in my day today |
@@ -112,6 +113,9 @@ func processorsGet(ctx *context.Context) []*Processor { | |||
} | |||
res := []*Processor{} | |||
for _, p := range procs { | |||
for _, c := range p.Cores { | |||
sort.Ints(c.LogicalProcessors) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't point my finger at anything but this looks suspicious. Let's monitor CI after we merge this PR. Not stopping because I can't back my gut feeling with any hard data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's just sorting the logical processor indexes to avoid situations like here where I thought there was a bug because the logical processors listed in a non-sorted way :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
thanks for your work! impressive
"nvme": StorageControllerNVMe, | ||
"virtio": StorageControllerVirtIO, | ||
"mmc": StorageControllerMMC, | ||
"loop": StorageControllerLoop, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when we use reverse table like this I'm not sure the backward compatibility is ensured, but I think this should not be a bother. We can break stuff moving to 1.0.0 (and then don't for a long long while)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't change any string constants. Only the exported names of the various constants (but kept the old ALL_CAPS ones so that existing installations that may have referenced those constant names won't break)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I misread that part. So we're golden!
doc and const cleanup for net package
doc and const cleanup for block package
ensure ProcessorCore.LogicalProcessors sorted
doc and const cleanup for memory package