Skip to content

Conversation

jaypipes
Copy link
Owner

All other fields that contain a total amount of something begin with the word Total except for the pkg/cpu.Processor.NumCores and NumThreads fields. This commit adds new fields to pkg/cpu.Processor called TotalCores and TotalHardwareThreads to bring the names of these fields in-line with all other amount/total fields.

In addition, adds a TotalHardwareThreads field to the pkg/cpu.Info struct and deprecates the less-explicit TotalThreads field.

I'd like to remove the now-deprecated NumThreads, NumCores, and TotalThreads fields before we get to a v.1.0 series.

All other fields that contain a total amount of something begin with the
word `Total` except for the `pkg/cpu.Processor.NumCores` and
`NumThreads` fields. This commit adds new fields to `pkg/cpu.Processor`
called `TotalCores` and `TotalHardwareThreads` to bring the names of
these fields in-line with all other amount/total fields.

In addition, adds a `TotalHardwareThreads` field to the `pkg/cpu.Info`
struct and deprecates the less-explicit `TotalThreads` field.

I'd like to remove the now-deprecated `NumThreads`, `NumCores`, and
`TotalThreads` fields before we get to a `v.1.0` series.

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
Copy link
Collaborator

@ffromani ffromani left a comment

Choose a reason for hiding this comment

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

LGTM.

  • we do change the docs, but that's fine, we're recommending the new fields
  • we keep setting correctly the old fields for backward compat

we do change the JSON representation though, do we need to test/guarantee the JSON roundtrip?
@jaypipes feel free to merge anytime if the JSON representation is not a concern for backward compat

@jaypipes
Copy link
Owner Author

LGTM.

* we do change the docs, but that's fine, we're recommending the new fields

* we keep setting correctly the old fields for backward compat

we do change the JSON representation though, do we need to test/guarantee the JSON roundtrip? @jaypipes feel free to merge anytime if the JSON representation is not a concern for backward compat

@ffromani yeah, see the issue I created yesterday about the JSON/YAML representation and firming that up with a published JSONSchema. :)

@jaypipes
Copy link
Owner Author

@ffromani also, note that I only added fields to the JSON output in this PR. It's still backwards-compat representation of the JSON/YAML but I do want to deprecate things before v1.0

@jaypipes jaypipes merged commit 2950821 into main Sep 25, 2024
14 checks passed
@jaypipes jaypipes deleted the num-total branch September 25, 2024 11:41
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