-
Notifications
You must be signed in to change notification settings - Fork 0
アーキテクチャ構成の修正案 #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
アーキテクチャ構成の修正案 #59
Conversation
毎回homeのconfを参照する件は、うまいことconfの情報をどこかにcacheできないのかなと思いつつ |
log.Fatalは |
serviceではconfの読み込みはしないというルールを設けてる |
ローカルでのメッセージ切り替え完了。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
恐らく議論できるだけの十分な知識は持ち合わせていないので、自分は他の用事も多くあまりコミットできないので、大島さんのこの構成で進めてもらって構いません。
@@ -18,7 +18,7 @@ var ( | |||
{}, | |||
{int(entity.EN), int(entity.JP)}, | |||
{int(entity.NormalFormat), int(entity.EmojiFormat), int(entity.PrefixFormat)}, | |||
{int(entity.WrapServer), int(entity.OpenAiAPI)}, | |||
{int(entity.Server), int(entity.Local)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここのentity.Local
が変数名間違えている?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
たしかに、修正する!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputOutput
という名前は抽象的すぎる気がする
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これをGitGatewayにしようかな
gitコマンドを操作してる感じだし
type GithubService interface { | ||
GetStaginCodeDiff() (string, error) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHubではなくて、単なるGitじゃない?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gitっていうツールではなく、GitHubっていう抽象的な概念に焦点当てた感じかな。
あとから、GithubのOAuthのメソッドもここにいれるつもりだった。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHubは概念ではなく、Gitのリモートリポジトリを管理する1サービスじゃない?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GithubがGitを包括するって意味で言いたかった!
でGithubServiceはGithubに関係するロジックをまとめたかったのが意図ではある。
わかりにくければGitにするわ
Merge pull request #59 from cocoide/feature/refactor-proposal
No description provided.