search

cncf-tags / container-device-interface

Apache License 2.0
200 stars 37 forks source link

Inconsistent naming of the specs-go module #227

Closed bart0sh closed 1 week ago

bart0sh commented 2 weeks ago 
$ git grep 'container-device-interface/specs-go"'
pkg/cdi/cache.go:       cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/cache_test.go:  cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/container-edits.go:     "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/container-edits_test.go:        cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/device.go:      cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/device_test.go: cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/doc.go://       "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/doc.go://       "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/registry.go:    cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/spec.go:        cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/spec_test.go:   cdi "tags.cncf.io/container-device-interface/specs-go"
pkg/cdi/validate/schema.go:     raw "tags.cncf.io/container-device-interface/specs-go"

I'd propose to replace all of that with unnamed import "tags.cncf.io/container-device-interface/specs-go"

klihub commented 2 weeks ago

Isn't that consistent naming ? I mean everywhere within pkg/cdi, we call cdi specs-go cdi, so the spec reference reads cdi.Spec, not specs.Spec. Similarly we call opencontainers/runtime-spec/spec-go oci, so we have then oci.Spec in the code.

Anyway we need to import one of them with a renamed alias, since we can't call both of them specs in the same file within a module. So then probably you would have specs.Spec instead of cdi.Spec and maybe oci.Spec or ocispec.Spec for the oci Spec. Personally I'd consider both of these less readable than the current implementation choice.

klihub commented 2 weeks ago

Ah ok. In pkg/container-edits.go we're inconsistent. If the current and above described reasoning is acceptable, then at least that should be changed to be consistent with the rest within pkg/cdi. And then I'd change raw to cdi in pkg/cdi/validate/schema.go as well.

klihub commented 2 weeks ago

Ah ok. In pkg/container-edits.go we're inconsistent... And then I'd change raw to cdi in pkg/cdi/validate/schema.go as well.

Filed #228 to fix those.

bart0sh commented 1 week ago

This seems to be closed by #228 /close