Repository: knative/pkg
Pull request #1837 (2020-10-23 10:39:05)

Fix API stutter in metric option functions
Before we roll those out, let's get rid of the new linting issues (API stuttering).

Also:
- Made the old functions deprecated.
- Fixed comments.
- Added comments to the new functions.

/assign @vagababov 

knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1837#" title="Author self-approved">markusthoemmes</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/master/metrics/OWNERS)~~ [markusthoemmes]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
knative-prow-robot: @markusthoemmes: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 39548015a9d8d53fd084d368ec1ed27eaed9a368 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1837/pull-knative-pkg-unit-tests/1319589542721753090) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1837). [Your PR dashboard](https://gubernator.knative.dev/pr/markusthoemmes).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @markusthoemmes: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 39548015a9d8d53fd084d368ec1ed27eaed9a368 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1837/pull-knative-pkg-unit-tests/1319589542721753090) | `/test pull-knative-pkg-unit-tests`
pull-knative-pkg-build-tests | 39548015a9d8d53fd084d368ec1ed27eaed9a368 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1837/pull-knative-pkg-build-tests/1319589542721753088) | `/test pull-knative-pkg-build-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1837). [Your PR dashboard](https://gubernator.knative.dev/pr/markusthoemmes).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow-robot: @markusthoemmes: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-build-tests | 39548015a9d8d53fd084d368ec1ed27eaed9a368 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1837/pull-knative-pkg-build-tests/1319589542721753088) | `/test pull-knative-pkg-build-tests`
pull-knative-pkg-unit-tests | e180764ffbb44e5f60e9a637b223086c5a43b6a2 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1837/pull-knative-pkg-unit-tests/1319597973616726018) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1837). [Your PR dashboard](https://gubernator.knative.dev/pr/markusthoemmes).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
vagababov: /lgtm
vagababov: /retest

Patch:
diff --git a/metrics/config.go b/metrics/config.go
index 1bdf06607..093d26d45 100644
--- a/metrics/config.go
+++ b/metrics/config.go
@@ -341,14 +341,15 @@ func prometheusPort() (int, error) {
 	return int(pp), nil
 }
 
-// JsonToMetricsOptions converts a json string of a
-// ExporterOptions. Returns a non-nil ExporterOptions always.
+// JsonToMetricsOptions converts a JSON string to an ExporterOptions object.
 // TODO(vagababov): remove after updating deps.
+// Deprecated: Use JSONToOptions instead.
 func JsonToMetricsOptions(jsonOpts string) (*ExporterOptions, error) { //nolint // No rename due to backwards incompatibility. {
-	return JSONToMetricsOptions(jsonOpts)
+	return JSONToOptions(jsonOpts)
 }
 
-func JSONToMetricsOptions(jsonOpts string) (*ExporterOptions, error) {
+// JSONToOptions converts a json string to ExporterOptions.
+func JSONToOptions(jsonOpts string) (*ExporterOptions, error) {
 	var opts ExporterOptions
 	if jsonOpts == "" {
 		return nil, errors.New("json options string is empty")
@@ -361,13 +362,15 @@ func JSONToMetricsOptions(jsonOpts string) (*ExporterOptions, error) {
 	return &opts, nil
 }
 
-// MetricsOptionsToJson converts a ExporterOptions to a json string.
+// MetricsOptionsToJson converts an ExporterOptions object to a JSON string.
 // TODO(vagababov): remove after updating deps.
+// Deprecated: Use OptionsToJSON instead.
 func MetricsOptionsToJson(opts *ExporterOptions) (string, error) { //nolint // No rename due to backwards incompatibility.
-	return MetricsOptionsToJSON(opts)
+	return OptionsToJSON(opts)
 }
 
-func MetricsOptionsToJSON(opts *ExporterOptions) (string, error) {
+// OptionsToJSON converts an ExporterOptions object to a JSON string.
+func OptionsToJSON(opts *ExporterOptions) (string, error) {
 	if opts == nil {
 		return "", nil
 	}


Test patch:
diff --git a/metrics/config_test.go b/metrics/config_test.go
index 5e8f881e6..8d47a5716 100644
--- a/metrics/config_test.go
+++ b/metrics/config_test.go
@@ -921,7 +921,7 @@ func TestMetricsOptions(t *testing.T) {
 	}
 	for n, tc := range testCases {
 		t.Run(n, func(t *testing.T) {
-			jsonOpts, err := MetricsOptionsToJSON(tc.opts)
+			jsonOpts, err := OptionsToJSON(tc.opts)
 			if err != nil {
 				t.Error("error while converting metrics config to json:", err)
 			}
@@ -937,7 +937,7 @@ func TestMetricsOptions(t *testing.T) {
 			// Test to options.
 			{
 				want := tc.opts
-				got, gotErr := JSONToMetricsOptions(jsonOpts)
+				got, gotErr := JSONToOptions(jsonOpts)
 
 				if gotErr != nil {
 					if diff := cmp.Diff(tc.wantErr, gotErr.Error()); diff != "" {