Repository: knative/pkg
Pull request #1848 (2020-10-26 20:46:15)

Add proper cased function to deprecate the old one
Json -> JSON
2 less linter errors!

/assign @tcnghia 

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1848#" title="Author self-approved">vagababov</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:

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

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: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1848#pullrequestreview-517162829" title="LGTM">tcnghia</a>*, *<a href="https://github.com/knative/pkg/pull/1848#" title="Author self-approved">vagababov</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:

- ~~[logging/OWNERS](https://github.com/knative/pkg/blob/master/logging/OWNERS)~~ [tcnghia,vagababov]

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":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=h1) Report
> Merging [#1848](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=desc) into [master](https://codecov.io/gh/knative/pkg/commit/c4ce2ffa6a3032ae4b665e4a3dbfa62eaebee45e?el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `0.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1848/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1848      +/-   ##
==========================================
- Coverage   68.21%   68.18%   -0.04%     
==========================================
  Files         222      222              
  Lines        9424     9426       +2     
==========================================
- Hits         6429     6427       -2     
- Misses       2673     2677       +4     
  Partials      322      322              
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [logging/config.go](https://codecov.io/gh/knative/pkg/pull/1848/diff?src=pr&el=tree#diff-bG9nZ2luZy9jb25maWcuZ28=) | `84.00% <0.00%> (-3.76%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=footer). Last update [c4ce2ff...3fe0b0b](https://codecov.io/gh/knative/pkg/pull/1848?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-prow-robot: @vagababov: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 3fe0b0b220f40d83ac681dea95a6f485c632b43e | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1848/pull-knative-pkg-unit-tests/1320829462761181186) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
yanweiguo: /lgtm
/retest
vagababov: /retest

Patch:
diff --git a/logging/config.go b/logging/config.go
index 9d4eda896..9f6183bdd 100644
--- a/logging/config.go
+++ b/logging/config.go
@@ -238,9 +238,16 @@ func ConfigMapName() string {
 	return "config-logging"
 }
 
-// JsonToLoggingConfig converts a json string of a Config.
-// Returns a non-nil Config always.
+// JsonToLoggingConfig converts a JSON string of a Config.
+// Always returns a non-nil Config.
+// TODO(vagababov): remove after downstream conversion.
 func JsonToLoggingConfig(jsonCfg string) (*Config, error) { //nolint No rename due to backwards incompatibility.
+	return JSONToConfig(jsonCfg)
+}
+
+// JSONToConfig converts a JSON string of a Config.
+// Always returns a non-nil Config.
+func JSONToConfig(jsonCfg string) (*Config, error) {
 	if jsonCfg == "" {
 		return nil, errEmptyJSONLogginString
 	}
@@ -258,8 +265,14 @@ func JsonToLoggingConfig(jsonCfg string) (*Config, error) { //nolint No rename d
 	return cfg, nil
 }
 
-// LoggingConfigToJson converts a Config to a json string.
+// LoggingConfigToJson converts a Config to a JSON string.
+// TODO(vagababov): remove after downstream conversion.
 func LoggingConfigToJson(cfg *Config) (string, error) { //nolint No rename due to backwards incompatibility.
+	return ConfigToJSON(cfg)
+}
+
+// ConfigToJSON  converts a Config to a JSON string.
+func ConfigToJSON(cfg *Config) (string, error) {
 	if cfg == nil || cfg.LoggingConfig == "" {
 		return "", nil
 	}


Test patch:
diff --git a/logging/config_test.go b/logging/config_test.go
index f917fdd95..1adb04524 100644
--- a/logging/config_test.go
+++ b/logging/config_test.go
@@ -407,33 +407,32 @@ func TestLoggingConfig(t *testing.T) {
 	}}
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			json, err := LoggingConfigToJson(tc.cfg)
+			json, err := ConfigToJSON(tc.cfg)
 			if err != nil {
-				t.Error("error while converting logging config to json:", err)
+				t.Error("Error while converting logging config to json:", err)
 			}
 			// Test to json.
-			{
-				want := tc.want
-				got := json
-				if diff := cmp.Diff(want, got); diff != "" {
-					t.Error("unexpected (-want, +got) =", diff)
+			t.Run("to JSON", func(t *testing.T) {
+				if got, want := json, tc.want; !cmp.Equal(got, want) {
+					t.Errorf("unexpected (-want, +got) =\n%s", cmp.Diff(want, got))
 				}
-			}
-			want := tc.cfg
-			got, gotErr := JsonToLoggingConfig(json)
-
-			if gotErr != nil {
-				if diff := cmp.Diff(tc.wantErr, gotErr.Error()); diff != "" {
-					t.Error("unexpected err (-want, +got) =", diff)
+			})
+			t.Run("from JSON", func(t *testing.T) {
+				want := tc.cfg
+				got, gotErr := JSONToConfig(tc.want)
+
+				if gotErr != nil {
+					if diff := cmp.Diff(tc.wantErr, gotErr.Error()); diff != "" {
+						t.Error("unexpected err (-want, +got) =", diff)
+					}
+				} else if tc.wantErr != "" {
+					t.Error("expected err", tc.wantErr)
 				}
-			} else if tc.wantErr != "" {
-				t.Error("expected err", tc.wantErr)
-			}
 
-			if diff := cmp.Diff(want, got); diff != "" {
-				t.Error("unexpected (-want, +got) =", diff)
-				t.Log(got)
-			}
+				if diff := cmp.Diff(want, got); diff != "" {
+					t.Errorf("Unexpected Config: (-want, +got) =\n%s", diff)
+				}
+			})
 		})
 	}
 }