feat: support pretty print json string #13
Conversation
internal/pretty_print/pretty_json.go
Outdated
| return old, nil | ||
| } | ||
|
|
||
| func copyPlan(old *tfjson.Plan) (*tfjson.Plan, error) { |
There was a problem hiding this comment.
[Q] deep copyが必要な理由ってなんでしょうか。そのまま書き換えるのだと駄目なんでしょうか。このデータ構造いじるのはterrraform-j2mdだけなのでオリジナルの方を保存しないといけないみたいなこともないんじゃないかと思いました。
There was a problem hiding this comment.
https://github.com/hashicorp/terraform-json/tree/main/sanitize と
実装を揃えてdeep copyするようにしています。個人的に明確に目的があってdeep copyしているわけではないです。
There was a problem hiding this comment.
sanitizeで使われているのはdeep copyじゃなくてshallow copyなので実装が揃っていなさそうであるのと、sanitizeの方は明確な意図があってだたのcopyじゃないのを選択しているので、明確な意図がないのであれば普通のコピーで実装できないか検討して欲しいです。
There was a problem hiding this comment.
あれ、shallow copyだ… なんかボケてたみたいです。対応します
There was a problem hiding this comment.
明確な意図がないのであれば普通のコピーで実装できないか検討して欲しいです。
実装合わせるんじゃなくて普通のコピーでできないかまずは検討して欲しいです
There was a problem hiding this comment.
コピーも必要なさそうだったので、オリジナルのデータをそのまま書き換えるようにしました。
internal/pretty_print/pretty_json.go
Outdated
| @@ -0,0 +1,97 @@ | |||
| package pretty_print | |||
There was a problem hiding this comment.
packages are given lower case, single-word names; there should be no need for underscores or mixedCaps
https://go.dev/doc/effective_go#names
goのパッケージ名にはアンダースコア入れないようです。
あと、printはしてないのでpretty_printじゃなくてprettyplanとかformatterとかformatの方が適切じゃないかなと思いました。もしくはterraformパッケージ内にこの関数書いてしまうとか。
internal/pretty_print/pretty_json.go
Outdated
|
|
||
| func prettyChangeValue(old interface{}) (interface{}, error) { | ||
| switch x := old.(type) { | ||
| case []interface{}: |
There was a problem hiding this comment.
[nits] 1.18からanyが使えるのでanyで良いんじゃないかと思いました。
internal/pretty_print/pretty_json.go
Outdated
| } | ||
| case string: | ||
| var j json.RawMessage | ||
| if err := json.Unmarshal([]byte(old.(string)), &j); err == nil && json.Valid([]byte(old.(string))) { |
There was a problem hiding this comment.
json.Validしてjson文字列じゃなかったらreturnして、json.Validがtrueならjson.Unmarshalするんじゃないかと思ったのですが、先にjson.Unmarshalしている理由は何でしょうか。
There was a problem hiding this comment.
明確な意図はないですね。先にValidを呼ぶ方が自然に思えるのでそうしようと思いました
|
すみません、通知の設定ミスで気づいていませんでした。確認します。 |
internal/terraform/plan.go
Outdated
| @@ -114,8 +115,12 @@ func NewPlanData(input []byte) (*PlanData, error) { | |||
| if err != nil { | |||
| return nil, fmt.Errorf("failed to sanitize plan: %w", err) | |||
| } | |||
| FormattedJsonPlan, err := format.FormatJsonPlan(sanitizedPlan) | |||
There was a problem hiding this comment.
[nits] local変数の始まりが大文字になってるのが気になりました。
internal/format/format_json_test.go
Outdated
| "testing" | ||
| ) | ||
|
|
||
| func Test_formatJsonChangeValue(t *testing.T) { |
There was a problem hiding this comment.
外に公開していないメソッドのテストしかないのが気になりました。FormatJsonPlanに対してテストを書ければ十分だと思うのでこの場合はFormatJsonPlanに対してテストを書いた方が良いと思いました。
また、テストのパッケージはできるだけxxx_testとして別のパッケージにして欲しいです。
ref. https://engineering.mercari.com/blog/entry/2018-08-08-080000/
There was a problem hiding this comment.
この記事は初めて読みましたが、確かにわけるメリットはありますね。
分割しました。
There was a problem hiding this comment.
動作確認しました。良さそうです 👍
go mod tidyを実行すると差分出るのでそれだけ最後修正お願いします。
git diff
diff --git a/go.mod b/go.mod
index b8c428f..1e71984 100644
--- a/go.mod
+++ b/go.mod
@@ -4,7 +4,6 @@ go 1.18
require (
github.com/hashicorp/terraform-json v0.14.0
- github.com/jinzhu/copier v0.3.5
github.com/pmezard/go-difflib v1.0.0
)
diff --git a/go.sum b/go.sum
index 7ffec86..f3fb121 100644
--- a/go.sum
+++ b/go.sum
@@ -13,8 +13,6 @@ github.com/hashicorp/go-version v1.5.0 h1:O293SZ2Eg+AAYijkVK3jR786Am1bhDEh2GHT0t
github.com/hashicorp/go-version v1.5.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
github.com/hashicorp/terraform-json v0.14.0 h1:sh9iZ1Y8IFJLx+xQiKHGud6/TSUCM0N8e17dKDpqV7s=
github.com/hashicorp/terraform-json v0.14.0/go.mod h1:5A9HIWPkk4e5aeeXIBbkcOvaZbIYnAIkEyqP2pNSckM=
-github.com/jinzhu/copier v0.3.5 h1:GlvfUwHk62RokgqVNvYsku0TATCF7bAHVwEXoBh3iJg=
-github.com/jinzhu/copier v0.3.5/go.mod h1:DfbEm0FYsaqBcKcFuvmOZb218JkPGtvSHsKg8S8hyyg=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
|
thank you! |
This PR supports to print json string as pretty.