Skip to content

アーキテクチャ構成の修正案 #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

Merged
merged 4 commits into from
Nov 6, 2023

Conversation

cocoide
Copy link
Owner

@cocoide cocoide commented Oct 5, 2023

No description provided.

@cocoide cocoide requested a review from mochi-yu October 5, 2023 15:54
@cocoide
Copy link
Owner Author

cocoide commented Oct 5, 2023

毎回homeのconfを参照する件は、うまいことconfの情報をどこかにcacheできないのかなと思いつつ
→ bubbleのcmdにキャッシュは可能かどうか

@cocoide
Copy link
Owner Author

cocoide commented Oct 5, 2023

log.Fatalは
os.Exit(1)を含むので
その後の処理は考えなくていいよ

@cocoide
Copy link
Owner Author

cocoide commented Oct 5, 2023

serviceではconfの読み込みはしないというルールを設けてる

@cocoide
Copy link
Owner Author

cocoide commented Oct 5, 2023

ローカルでのメッセージ切り替え完了。
DI元を切り替えることで実装。

{cocoide} added 2 commits October 6, 2023 02:20
Copy link
Collaborator

@mochi-yu mochi-yu left a 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)},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここのentity.Localが変数名間違えている?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかに、修正する!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputOutputという名前は抽象的すぎる気がする

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これをGitGatewayにしようかな
gitコマンドを操作してる感じだし

Comment on lines +11 to +13
type GithubService interface {
GetStaginCodeDiff() (string, error)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHubではなくて、単なるGitじゃない?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitっていうツールではなく、GitHubっていう抽象的な概念に焦点当てた感じかな。
あとから、GithubのOAuthのメソッドもここにいれるつもりだった。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHubは概念ではなく、Gitのリモートリポジトリを管理する1サービスじゃない?

Copy link
Owner Author

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にするわ

@cocoide cocoide merged commit 222269c into feature/57-refactor-file-arch Nov 6, 2023
cocoide added a commit that referenced this pull request Nov 6, 2023
Merge pull request #59 from cocoide/feature/refactor-proposal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants