refactor(lib): Remove environment variable dependencies from lib.List() #118
Conversation
MH4GF
changed the title
| os.Exit(1) | ||
| } | ||
|
|
||
| fmt.Println(lines) |
There was a problem hiding this comment.
linesのstdout出力は元々 lib.List() 内で行われていましたが、ライブラリとして利用する際はstdoutではなくstringを受け取って操作したいです。
そのため呼び出し側でstdout出力するようリファクタリングしました。
| accessToken: accessToken, | ||
| settingsGistID: settingsGistID, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
既存のCLIとしてのユースケースをサポートする NewListFromCLI() を用意しました。 今までと同様に環境変数やgit configから認証情報等を取得しListを返します。
There was a problem hiding this comment.
なるほど。素朴に cmd/root.go にベタ書きすることを考えてましたが、ビジネスロジック（大げさ）は lib/ にあったほうが良さそうですね。
There was a problem hiding this comment.
getAccessToken()などはプライベートメソッドだったので、パッケージ外に公開するのはな......と考えこの形にしました 😄
ただ、逆にCLIからしか使われないロジックはcmdパッケージに寄せるのもありかも？とも思ったり 🤔 もしくはcli的なパッケージを切るのもありかもです(やりすぎかもですが)
There was a problem hiding this comment.
あ、そうか。異なるパッケージからは呼べませんでしたね 😅
話逸れますが、C 言語の static 関数はそのファイル内からしか呼べなかったので、ファイル単位でパッケージ化できて、個人的には好きでした。
Go だとディレクトリを分けないといけないのがなあ...。
There was a problem hiding this comment.
おおー、そうなんですね、よさそうですね！RubyやJSだと逆に一度公開しちゃうとどこからでも呼べちゃうのがな...と思ってました 😅
| log.Fatal(err) | ||
| return "", err |
There was a problem hiding this comment.
log.Fatal() は 内部的に os.Exit(1) してしまうため、ライブラリとしてのユースケースでは困ってしまいます。
そのため単純にerrを返すよう修正しました。
| if err = settings.Init(getGistID(), accessToken); err != nil { | ||
| return err | ||
| if err = settings.Init(l.settingsGistID, l.accessToken); err != nil { | ||
| return "", err | ||
| } | ||
| format := NewFormat(ctx, client, settings, debug) | ||
|
|
||
| parallelNum, err := getParallelNum() |
There was a problem hiding this comment.
NOTE: 将来的にはここや、タイムゾーンなども外部から注入できるようにしたいですが、必要に応じてPRを出すつもりです。
|
🚀 v4.2.7 |
Settings.Init()andFormat.All()#116ライブラリとしてのユースケースをサポートするために、各メソッドから環境変数やgit configへの依存を取り除いていきます。
今回は本丸の
lib.List()の対応となります。Listをstructに変更しフィールドを外部から注入可能にしました。動作確認方法
以下の内容を確認しています。
make test-all,make lintが通ることmake distを実施し、./pkg/darwin_arm64/github-nippouが問題なく動作すること