k8stopologyawareschedwg / noderesourcetopology-api

This repository contains an implementation for CRD (https://docs.google.com/document/d/12kj3fK8boNuPNqob6F_pPU9ZTaNEnPGaXEooW1Cilwg/edit)
Apache License 2.0
8 stars 6 forks source link

introduce the API helpers package #31

Closed ffromani closed 1 year ago

ffromani commented 1 year ago

add helpers to consolidate utility code commonly used by consumers of the API. modeled after https://github.com/kubernetes/kubernetes/tree/v1.26.1/pkg/apis/core/v1/helper

ffromani commented 1 year ago

FYI @marquiz @PiotrProkop

PiotrProkop commented 1 year ago

LGTM! Good job. 🚀

ffromani commented 1 year ago

Makefile additions deferred to #33

ffromani commented 1 year ago

Updated. The only remaining concern is attaching or not the behavior to the objects. Still investigating.

ffromani commented 1 year ago

I'm inclined to follow the lead of https://github.com/kubernetes/kubernetes/tree/master/pkg/apis/core/v1/helper and don't attach behavior to API objects, using free functions. We can totally revisit this decision if there is new evidence or guidelines

marquiz commented 1 year ago

I'm inclined to follow the lead of https://github.com/kubernetes/kubernetes/tree/master/pkg/apis/core/v1/helper and don't attach behavior to API objects, using free functions.

I'm inclined to agree with this 😇

ffromani commented 1 year ago

I'm inclined to follow the lead of https://github.com/kubernetes/kubernetes/tree/master/pkg/apis/core/v1/helper and don't attach behavior to API objects, using free functions.

I'm inclined to agree with this innocent

Thanks! I want to emphasize this decision is not final and I'm very open to pivot to the other direction if new evidence or recommendations comes out. We have more wiggle room wrt the API guarantees (and API is still v1alphaX anyway!)

ffromani commented 1 year ago

it seems we have consensus, so I'm merging!