Skip to content

Commit 1bddc48

Browse files
committed
Address code review feedback: simplify annotations and add TODOs
- Simplify shortName for TemporalConnection to just "tconn" - Remove print column annotations and replace with TODO comments linking to k8s conventions - Add TODO comments for unit test cases on reference types Per code review feedback on PR #136.
1 parent b5616c6 commit 1bddc48

2 files changed

Lines changed: 7 additions & 8 deletions

File tree

api/v1alpha1/temporalconnection_types.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
1212

1313
// SecretReference contains the name of a Secret resource in the same namespace.
14+
// TODO(jlegrone): Add table-driven unit test cases for this
1415
type SecretReference struct {
1516
// Name of the Secret resource.
1617
// +kubebuilder:validation:Required
@@ -51,10 +52,9 @@ type TemporalConnectionStatus struct {
5152

5253
//+kubebuilder:object:root=true
5354
//+kubebuilder:subresource:status
54-
// +kubebuilder:resource:shortName=tc;tconn;temporalconn
55-
//+kubebuilder:printcolumn:name="Host",type="string",JSONPath=".spec.hostPort",description="Temporal server host and port"
56-
//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status",description="Connection readiness status"
57-
//+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
55+
// +kubebuilder:resource:shortName=tconn
56+
// TODO(jlegrone): Add print column annotations following Kubernetes conventions
57+
// https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata
5858

5959
// TemporalConnection is the Schema for the temporalconnections API
6060
type TemporalConnection struct {

api/v1alpha1/worker_types.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
// TemporalConnectionReference contains the name of a TemporalConnection resource
1515
// in the same namespace as the TemporalWorkerDeployment.
16+
// TODO(jlegrone): Add table-driven unit test cases for this
1617
type TemporalConnectionReference struct {
1718
// Name of the TemporalConnection resource.
1819
// +kubebuilder:validation:Required
@@ -336,10 +337,8 @@ type ManualRolloutStrategy struct{}
336337
//+kubebuilder:object:root=true
337338
//+kubebuilder:subresource:status
338339
// +kubebuilder:resource:shortName=twd;twdeployment;tworkerdeployment
339-
//+kubebuilder:printcolumn:name="Current",type="string",JSONPath=".status.currentVersion.buildID",description="Current Version Build ID"
340-
//+kubebuilder:printcolumn:name="Target",type="string",JSONPath=".status.targetVersion.buildID",description="Build ID of the target worker (based on the pod template)"
341-
//+kubebuilder:printcolumn:name="Target-Ramp",type="number",JSONPath=".status.targetVersion.rampPercentageBasisPoints",description="Basis points of new workflows starting on Target Version (100 basis points = 1%)"
342-
//+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
340+
// TODO(jlegrone): Add print column annotations following Kubernetes conventions
341+
// https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata
343342

344343
// TemporalWorkerDeployment is the Schema for the temporalworkerdeployments API
345344
type TemporalWorkerDeployment struct {

0 commit comments

Comments
 (0)