feat: Awesome Yasunori MCP #171
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- packages/mcp/package.json: Language not supported
- packages/mcp/tsconfig.json: Language not supported
Comments suppressed due to low confidence (3)
packages/mcp/build.config.ts:14
- Consider using a path handling utility (e.g., path.resolve) to construct the executable path, ensuring it works correctly in various environments.
child_process.execSync(`chmod +x ./${bin}`);
packages/mcp/src/index.test.ts:110
- The expected output in this test appears to rely on a very specific hardcoded message that does not match the tool response implementation; please verify and update the expected output accordingly.
text: expect.stringContaining(JSON.stringify("tomoyaさん、ありすえさんこんにちは。\nはじめまして、yasunoriの母です。\n\nyasunoriがソフトウェアエンジニアを志してから様子がおかしくなってしまいました。\n家ですれ違う時「Vim....Vim....」という独り言をずっと唱えていたり、部屋からは「設定させていただきありがとうございます!!」という大声が聞こえてきたり、\n「会合があるから東京に行ってくる、帰りは遅くなる」と言い残して出て行き、帰ってくると満面の笑みで「Vimはいいぞ」と一言言って自室に篭るようになりました。\n\ntomoyaさんありすえさんもVimコミュニティの人達だと伺いましたが、息子の身に一体何が起きてしまったのか教えていただけると幸いです。\n"))
packages/mcp/src/index.ts:54
- [nitpick] The dynamic property access using [":id"] can be confusing; consider a clearer naming or accessor method if the API allows.
const res = await client.awesome[":id"].$get({
Co-authored-by: Copilot <[email protected]>
tomoya
requested changes
Apr 7, 2025
kuuote
approved these changes
Apr 7, 2025
|
テスト修正しました！ |
|
ビルド変更したので @kuuote さんにも再レビューをお願いするなどします |
tomoya
requested changes
Apr 8, 2025
packages/mcp/src/index.test.ts
Outdated
| @@ -0,0 +1,110 @@ | |||
| import { Client } from "@modelcontextprotocol/sdk/client/index.js"; | |||
| import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; | |||
| import { describe, expect, it } from "vitest"; | |||
There was a problem hiding this comment.
現状、it は never read のはず。
https://vitest.dev/config/#globals
ミスも増えそうなので、設定で global: true にしておいたほうがいいと思いました。
あと、typecheck を実行したほうがいいかも?
There was a problem hiding this comment.
サクッと作りたかったのであえて今回はglobalsを設定しませんでした。設定ファイルが一つ増えるのめんどくさいなと思ってしまったので
今回みたいな小さいプロジェクトでもglobalの設定は必要ですか？
設定しなくてもvitestがそのまま動くのがいいかなと思ってました
|
レビューありがとうございました！ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing Awesome Yasunori MCP Server!!!
ay-m.mp4