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

use Go idiomatic naming for Architecture consts #379

Merged
merged 1 commit into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,12 @@ var (
type Architecture = topology.Architecture

const (
ARCHITECTURE_SMP = topology.ARCHITECTURE_SMP
ARCHITECTURE_NUMA = topology.ARCHITECTURE_NUMA
ArchitectureSMP = topology.ArchitectureSMP
// DEPRECATED: Please use ArchitectureSMP
ARCHITECTURE_SMP = topology.ArchitectureSMP
ArchitectureNUMA = topology.ArchitectureNUMA
// DEPRECATED: Please use ArchitectureNUMA
ARCHITECTURE_NUMA = topology.ArchitectureNUMA
)

type PCIInfo = pci.Info
Expand Down
2 changes: 1 addition & 1 deletion pkg/gpu/gpu_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func gpuFillNUMANodes(ctx *context.Context, cards []*GraphicsCard) {
// Problem getting topology information so just set the graphics card's
// node to nil
for _, card := range cards {
if topo.Architecture != topology.ARCHITECTURE_NUMA {
if topo.Architecture != topology.ArchitectureNUMA {
card.Node = nil
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/pci/pci.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func New(opts ...*option.Option) (*Info, error) {
// by default we don't report NUMA information;
// we will only if are sure we are running on NUMA architecture
info := &Info{
arch: topology.ARCHITECTURE_SMP,
arch: topology.ArchitectureSMP,
ctx: ctx,
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/pci/pci_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ func (info *Info) GetDevice(address string) *Device {

device := info.getDeviceFromModaliasInfo(address, modaliasInfo)
device.Revision = getDeviceRevision(info.ctx, pciAddr)
if info.arch == topology.ARCHITECTURE_NUMA {
if info.arch == topology.ArchitectureNUMA {
device.Node = getDeviceNUMANode(info.ctx, pciAddr)
}
device.Driver = getDeviceDriver(info.ctx, pciAddr)
Expand Down
23 changes: 16 additions & 7 deletions pkg/topology/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,24 @@ type Architecture int

const (
// SMP is a Symmetric Multi-Processor system
ARCHITECTURE_SMP Architecture = iota
ArchitectureSMP Architecture = iota
// NUMA is a Non-Uniform Memory Access system
ARCHITECTURE_NUMA
ArchitectureNUMA
Comment on lines +29 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC we do allow importing sub-packages (import github.com/jaypipes/ghw/pkg/topology) and these are part of the public API. If so we need the backward compatibility also here

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ack, makes sense. I'll add the pkg/-specific old names back in, with deprecation warnings.

Copy link
Owner Author

Choose a reason for hiding this comment

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

done.

)

const (
// DEPRECATED: please use ArchitectureSMP.
// TODO(jaypipes): Remove before v1.0
ARCHITECTURE_SMP = ArchitectureSMP
// DEPRECATED: please use ArchitectureNUMA.
// TODO(jaypipes): Remove before v1.0
ARCHITECTURE_NUMA = ArchitectureNUMA
)

var (
architectureString = map[Architecture]string{
ARCHITECTURE_SMP: "SMP",
ARCHITECTURE_NUMA: "NUMA",
ArchitectureSMP: "SMP",
ArchitectureNUMA: "NUMA",
}

// NOTE(fromani): the keys are all lowercase and do not match
Expand All @@ -43,8 +52,8 @@ var (
// Architecture:MarshalJSON.
// We use this table only in UnmarshalJSON, so it should be OK.
stringArchitecture = map[string]Architecture{
"smp": ARCHITECTURE_SMP,
"numa": ARCHITECTURE_NUMA,
"smp": ArchitectureSMP,
"numa": ArchitectureNUMA,
}
)

Expand Down Expand Up @@ -126,7 +135,7 @@ func New(opts ...*option.Option) (*Info, error) {

func (i *Info) String() string {
archStr := "SMP"
if i.Architecture == ARCHITECTURE_NUMA {
if i.Architecture == ArchitectureNUMA {
archStr = "NUMA"
}
res := fmt.Sprintf(
Expand Down
4 changes: 2 additions & 2 deletions pkg/topology/topology_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ import (
func (i *Info) load() error {
i.Nodes = topologyNodes(i.ctx)
if len(i.Nodes) == 1 {
i.Architecture = ARCHITECTURE_SMP
i.Architecture = ArchitectureSMP
} else {
i.Architecture = ARCHITECTURE_NUMA
i.Architecture = ArchitectureNUMA
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/topology/topology_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestTopology(t *testing.T) {
t.Fatalf("Expected >0 nodes but got 0.")
}

if info.Architecture == topology.ARCHITECTURE_NUMA && len(info.Nodes) == 1 {
if info.Architecture == topology.ArchitectureNUMA && len(info.Nodes) == 1 {
t.Fatalf("Got NUMA architecture but only 1 node.")
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/topology/topology_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ func (i *Info) load() error {
}
i.Nodes = nodes
if len(nodes) == 1 {
i.Architecture = ARCHITECTURE_SMP
i.Architecture = ArchitectureSMP
} else {
i.Architecture = ARCHITECTURE_NUMA
i.Architecture = ArchitectureNUMA
}
return nil
}
Expand Down
Loading