-
Notifications
You must be signed in to change notification settings - Fork 2
README: Improve unboxing experience #23
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
Conversation
Caution Review failedThe pull request is closed. WalkthroughThis update revises and expands project documentation, including a comprehensive rewrite of the README.md, significant additions to the backlog planning document, and the introduction of a new example shell script for using Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant mcptools.sh
participant MCP Server
User->>mcptools.sh: Run example script
mcptools.sh->>MCP Server: mcpt query_sql, get_table_metadata, get_health, get_cratedb_documentation_index, fetch_cratedb_docs
MCP Server-->>mcptools.sh: Returns query results / metadata / documentation
mcptools.sh-->>User: Displays results
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (6)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (8)
docs/backlog.md (3)
3-8
: Add hyphens for list consistencyThe URL line isn’t prefixed as a bullet, which breaks markdown list formatting. Prefix it with a hyphen or indent it properly under the surrounding list:
- https://fluxcd.control-plane.io/mcp/config/ + - https://fluxcd.control-plane.io/mcp/config/
13-15
: Consistent bullet formatting for URLsThe link under "Install: Others also just use
docker
right away?" isn’t prefixed as a list item. Add a leading hyphen to ensure it renders correctly:- https://github.com/ppl-ai/modelcontextprotocol/?tab=readme-ov-file#step-3-configure-claude-desktop + - https://github.com/ppl-ai/modelcontextprotocol/?tab=readme-ov-file#step-3-configure-claude-desktop
29-32
: Indent sub-items under "Provide "instructions""The
- Blueprint:
line and its nested URLs should be indented under the "Provide "instructions"" bullet for correct markdown nesting. For example:- - Blueprint: + - Blueprint:examples/mcptools.sh (1)
1-3
: Add strict mode and specify shellFor better error handling and portability, consider using:
- #!/usr/bin/env sh + #!/usr/bin/env bash + set -euo pipefailThis ensures the script exits on errors, treats unset variables as failures, and propagates pipeline errors.
README.md (4)
24-26
: Hyphenate compound adjectiveConsider hyphenating "natural language" when used as a modifier:
-The CrateDB MCP Server for natural language Text-to-SQL and documentation +The CrateDB MCP Server for natural-language Text-to-SQL and documentation retrieval specializes in CrateDB database clusters.
55-56
: Add missing preposition for clarityUpdate the phrase to read more naturally:
- The [installation docs](#install) section includes guidelines how to - install it on your machine. + The [installation docs](#install) section includes guidelines on how to + install it on your machine.🧰 Tools
🪛 LanguageTool
[uncategorized] ~55-~55: Possible missing preposition found.
Context: ...](#install) section includes guidelines how to install it on your machine. #### Cl...(AI_EN_LECTOR_MISSING_PREPOSITION)
206-206
: Use question mark for a questionThis item is phrased as a question. Replace the trailing period with a question mark:
- * How can I format a timestamp column to '2019 Jan 21'. + * How can I format a timestamp column to '2019 Jan 21'?🧰 Tools
🪛 LanguageTool
[typographical] ~206-~206: Except for inverted sentences, ‘can I’ requires a question mark at the end of the sentence.
Context: ...rmat a timestamp column to '2019 Jan 21'. Please also explore the [example quest...(MD_PRP_QUESTION_MARK)
217-217
: Hyphenate compound adjectiveIn "open source project," add a hyphen for the compound modifier:
- The `cratedb-mcp` package is an open source project, and is [managed on + The `cratedb-mcp` package is an open-source project, and is [managed on🧰 Tools
🪛 LanguageTool
[uncategorized] ~217-~217: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ibuting Thecratedb-mcp
package is an open source project, and is [managed on GitHub]. Co...(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(5 hunks)docs/backlog.md
(2 hunks)examples/mcptools.sh
(1 hunks)pyproject.toml
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
examples/mcptools.sh (2)
cratedb_mcp/__main__.py (5)
query_sql
(28-31)get_table_metadata
(54-61)get_health
(65-67)get_cratedb_documentation_index
(38-39)fetch_cratedb_docs
(46-50)cratedb_mcp/cli.py (1)
serve
(44-50)
🪛 LanguageTool
README.md
[uncategorized] ~55-~55: Possible missing preposition found.
Context: ...](#install) section includes guidelines how to install it on your machine. #### Cl...
(AI_EN_LECTOR_MISSING_PREPOSITION)
[typographical] ~206-~206: Except for inverted sentences, ‘can I’ requires a question mark at the end of the sentence.
Context: ...rmat a timestamp column to '2019 Jan 21'. Please also explore the [example quest...
(MD_PRP_QUESTION_MARK)
[uncategorized] ~217-~217: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ibuting The cratedb-mcp
package is an open source project, and is [managed on GitHub]. Co...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (2)
pyproject.toml (1)
10-10
: Description update looks good.The revised project description clearly specifies "natural language Text-to-SQL and documentation retrieval," which improves clarity.
README.md (1)
1-5
: Strong overall improvementsThe addition of a structured "Quickstart Guide" and expanded "Handbook" sections significantly enhances clarity and smooths the onboarding experience.
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
README.md (1)
245-245
:⚠️ Potential issueCritical: fix the Releases link URL.
The[Releases]
reference points github.com/surister/cratedb-mcp/releases
but the org iscrate/cratedb-mcp
. Without correction, the link is broken.-[Releases]: https://github.com/surister/cratedb-mcp/releases +[Releases]: https://github.com/crate/cratedb-mcp/releases
🧹 Nitpick comments (9)
docs/backlog.md (3)
10-22
: Consider converting bare URLs to reference-style links and normalizing list indentation.
Markdownlint flags several bare URLs (MD034) and inconsistent sub-list indentation (MD007). To improve readability and maintain consistency, you can define link references at the bottom and indent nested items by two spaces.- - https://gofastmcp.com/getting-started/welcome#fastmcp-2-0-and-the-official-mcp-sdk + - [FastMCP 2.0 getting-started guide][fastmcp-2.0] ... -at https://code.visualstudio.com/docs/copilot/chat/mcp-servers#_add-an-mcp-server-to-your-workspace + - [VS Code MCP servers guide][vscode-mcp] ...And at the bottom of
docs/backlog.md
:+ [fastmcp-2.0]: https://gofastmcp.com/getting-started/welcome#fastmcp-2-0-and-the-official-mcp-sdk + [vscode-mcp]: https://code.visualstudio.com/docs/copilot/chat/mcp-servers#_add-an-mcp-server-to-your-workspaceAdjust other URLs similarly.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Bare URL used
null(MD034, no-bare-urls)
12-12: Bare URL used
null(MD034, no-bare-urls)
14-14: Bare URL used
null(MD034, no-bare-urls)
16-16: Bare URL used
null(MD034, no-bare-urls)
17-17: Bare URL used
null(MD034, no-bare-urls)
18-18: Bare URL used
null(MD034, no-bare-urls)
19-19: Bare URL used
null(MD034, no-bare-urls)
21-21: Bare URL used
null(MD034, no-bare-urls)
22-22: Bare URL used
null(MD034, no-bare-urls)
25-39
: Unify quotation and bullet formatting in Iteration +3.
The bullet- Provide "instructions"
mixes double quotes inside double-quoted text; consider using backticks or single quotes for clarity. Also, sub-bullets under “Blueprint:” should be indented consistently (two spaces).- - Provide "instructions" + - Provide instructions (from `instructions.md`) ... - - https://fluxcd.io/blog/2025/05/ai-assisted-gitops/ + - [AI-assisted GitOps blog][flux-gitops]Apply this pattern for the other Blueprint links.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
31-31: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
31-31: Bare URL used
null(MD034, no-bare-urls)
32-32: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
32-32: Bare URL used
null(MD034, no-bare-urls)
33-33: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
33-33: Bare URL used
null(MD034, no-bare-urls)
47-59
: Align Done section bullets and URLs.
Several “Docs:” entries use block quotes and bare URLs; choose one style (e.g., reference links) and indent consistently under their parent bullet. This will satisfy markdownlint MD034 and MD007.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Bare URL used
null(MD034, no-bare-urls)
54-54: Bare URL used
null(MD034, no-bare-urls)
59-59: Bare URL used
null(MD034, no-bare-urls)
README.md (6)
24-28
: Normalize hyphenation for “natural language”.
Lines 24–25 use “natural-language Text-to-SQL” (hyphenated) but line 34 uses “natural language” (no hyphen). For consistency, choose one form—e.g., “natural-language Text-to-SQL and documentation retrieval specializes...”
30-42
: Tighten the Introduction phrasing and heading spacing.
The new “### Introduction” is great, but there’s an extra blank line and slight redundancy in describing the MCP Server. Consider merging lines and ensuring one blank line before the heading. Also, “natural language” vs. “natural-language” should match the About section.
44-56
: Refine Quickstart client list ordering and naming.
The list of MCP clients is comprehensive but could be alphabetized or grouped by category (e.g., “OpenAI” vs. “VS Code”). Also verify the spelling of “Cline” vs. “Cline Bot”. This helps users scan supported clients more easily.
118-122
: Replace inline bare URL with reference link.
Line 120 uses<https://cratedb.com/docs>
. To satisfy MD034 and improve readability, convert it to a reference-style link, for example:- Relevant information is pulled from <https://cratedb.com/docs>, curated per + Relevant information is pulled from [CrateDB Docs][cratedb-docs], curated per ... + [cratedb-docs]: https://cratedb.com/docs
135-140
: Grammar: use “recommend using” instead of “recommend to use”.
Lines 137–140 and 153 read “We recommend to useuv tool install
…” Consider updating to “We recommend usinguv tool install
…” for smoother English.- We recommend to use `uv tool install` to install the `cratedb-mcp` package… + We recommend using `uv tool install` to install the `cratedb-mcp` package…Also applies to: 153-153
203-206
: Unify unordered list style: use dashes instead of asterisks.
The example questions at lines 203–206 use*
bullets, whereas the rest of the document uses-
. Switch to-
to satisfy MD004.- * Optimize this query: "SELECT * FROM movies WHERE release_date > '2012-12-1' AND revenue" - * Tell me about the health of the cluster - * What is the storage consumption of my tables, give it in a graph. - * How can I format a timestamp column to '2019 Jan 21'? + - Optimize this query: "SELECT * FROM movies WHERE release_date > '2012-12-1' AND revenue" + - Tell me about the health of the cluster + - What is the storage consumption of my tables, give it in a graph. + - How can I format a timestamp column to '2019 Jan 21'?🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
203-203: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
204-204: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
205-205: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
206-206: Unordered list style
Expected: dash; Actual: asterisk(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(5 hunks)docs/backlog.md
(2 hunks)pyproject.toml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
203-203: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
204-204: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
205-205: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
206-206: Unordered list style
Expected: dash; Actual: asterisk
(MD004, ul-style)
docs/backlog.md
6-6: Bare URL used
null
(MD034, no-bare-urls)
11-11: Bare URL used
null
(MD034, no-bare-urls)
12-12: Bare URL used
null
(MD034, no-bare-urls)
14-14: Bare URL used
null
(MD034, no-bare-urls)
16-16: Bare URL used
null
(MD034, no-bare-urls)
17-17: Bare URL used
null
(MD034, no-bare-urls)
18-18: Bare URL used
null
(MD034, no-bare-urls)
19-19: Bare URL used
null
(MD034, no-bare-urls)
21-21: Bare URL used
null
(MD034, no-bare-urls)
22-22: Bare URL used
null
(MD034, no-bare-urls)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Bare URL used
null
(MD034, no-bare-urls)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Bare URL used
null
(MD034, no-bare-urls)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Bare URL used
null
(MD034, no-bare-urls)
49-49: Bare URL used
null
(MD034, no-bare-urls)
54-54: Bare URL used
null
(MD034, no-bare-urls)
59-59: Bare URL used
null
(MD034, no-bare-urls)
61-61: Bare URL used
null
(MD034, no-bare-urls)
62-62: Bare URL used
null
(MD034, no-bare-urls)
🔇 Additional comments (1)
docs/backlog.md (1)
4-5
: Iteration +1 tasks look good.
The new backlog items for read-only user guidelines and CLI flag honor are clear and aligned with upcoming v0.0.1 release planning.
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/backlog.md (4)
4-6
: Convert bare URL to descriptive markdown link
The link on line 6 is a bare URL (https://fluxcd.control-plane.io/mcp/config/
), which triggers MD034. Consider replacing it with an inline link or reference-style link for clarity and lint compliance.Example diff:
- - https://fluxcd.control-plane.io/mcp/config/ + - [Flux MCP configuration guide](https://fluxcd.control-plane.io/mcp/config/)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Bare URL used
null(MD034, no-bare-urls)
10-23
: Use descriptive link text instead of bare URLs
Several items under Iteration +2 (lines 11, 12, 14, 16–19, 21–22) are bare URLs. For better readability and to satisfy MD034, replace each with a descriptive link. You can also define them as reference-style links at the bottom of this section if preferred.For example:
- - https://gofastmcp.com/getting-started/welcome#fastmcp-2-0-and-the-official-mcp-sdk + - [FastMCP 2.0 getting started guide](https://gofastmcp.com/getting-started/welcome#fastmcp-2-0-and-the-official-mcp-sdk)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
11-11: Bare URL used
null(MD034, no-bare-urls)
12-12: Bare URL used
null(MD034, no-bare-urls)
14-14: Bare URL used
null(MD034, no-bare-urls)
16-16: Bare URL used
null(MD034, no-bare-urls)
17-17: Bare URL used
null(MD034, no-bare-urls)
18-18: Bare URL used
null(MD034, no-bare-urls)
19-19: Bare URL used
null(MD034, no-bare-urls)
21-21: Bare URL used
null(MD034, no-bare-urls)
22-22: Bare URL used
null(MD034, no-bare-urls)
25-33
: Fix list indentation and link formatting in “Blueprint” block
The nested list under Blueprint: (lines 31–33) has inconsistent indentation (MD007) and uses bare URLs (MD034). Consider unindentingBlueprint:
to the same level as other bullets and then nesting links two spaces deeper. Also convert each URL to a descriptive link.Example diff:
- Blueprint: - - https://fluxcd.io/blog/2025/05/ai-assisted-gitops/ - - https://fluxcd.control-plane.io/mcp/prompt-engineering/ - - https://raw.githubusercontent.com/.../instructions.md + - Blueprint: + - [AI-assisted GitOps blog post](https://fluxcd.io/blog/2025/05/ai-assisted-gitops/) + - [FluxCD prompt-engineering guide](https://fluxcd.control-plane.io/mcp/prompt-engineering/) + - [MCP server instructions.md](https://raw.githubusercontent.com/controlplaneio-fluxcd/distribution/refs/heads/main/docs/mcp/instructions.md)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
31-31: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
31-31: Bare URL used
null(MD034, no-bare-urls)
32-32: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
32-32: Bare URL used
null(MD034, no-bare-urls)
33-33: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
33-33: Bare URL used
null(MD034, no-bare-urls)
48-62
: Standardize link formatting in the “Done” section
The Done block contains multiple bare URLs (lines 49, 54, 59, 61–62). For consistency and lint compliance, convert these to inline or reference links. For example:- - CLI: For real. Blueprint: https://fluxcd.control-plane.io/mcp/config/ + - CLI: For real. Blueprint: [Flux MCP config](https://fluxcd.control-plane.io/mcp/config/)And similarly update blockquotes:
- > -- https://fluxcd.control-plane.io/mcp/install/#configuration-with-ai-assistants + > -- [AI assistant configuration instructions](https://fluxcd.control-plane.io/mcp/install/#configuration-with-ai-assistants)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
49-49: Bare URL used
null(MD034, no-bare-urls)
54-54: Bare URL used
null(MD034, no-bare-urls)
59-59: Bare URL used
null(MD034, no-bare-urls)
61-61: Bare URL used
null(MD034, no-bare-urls)
62-62: Bare URL used
null(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(5 hunks)docs/backlog.md
(2 hunks)pyproject.toml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- README.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/backlog.md
6-6: Bare URL used
null
(MD034, no-bare-urls)
11-11: Bare URL used
null
(MD034, no-bare-urls)
12-12: Bare URL used
null
(MD034, no-bare-urls)
14-14: Bare URL used
null
(MD034, no-bare-urls)
16-16: Bare URL used
null
(MD034, no-bare-urls)
17-17: Bare URL used
null
(MD034, no-bare-urls)
18-18: Bare URL used
null
(MD034, no-bare-urls)
19-19: Bare URL used
null
(MD034, no-bare-urls)
21-21: Bare URL used
null
(MD034, no-bare-urls)
22-22: Bare URL used
null
(MD034, no-bare-urls)
31-31: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
31-31: Bare URL used
null
(MD034, no-bare-urls)
32-32: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
32-32: Bare URL used
null
(MD034, no-bare-urls)
33-33: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
33-33: Bare URL used
null
(MD034, no-bare-urls)
49-49: Bare URL used
null
(MD034, no-bare-urls)
54-54: Bare URL used
null
(MD034, no-bare-urls)
59-59: Bare URL used
null
(MD034, no-bare-urls)
61-61: Bare URL used
null
(MD034, no-bare-urls)
62-62: Bare URL used
null
(MD034, no-bare-urls)
@coderabbitai: Thanks for all the suggestions. You don't need to mention chore/formatting shortcomings in |
This comment was marked as duplicate.
This comment was marked as duplicate.
Tip For best results, initiate chat on the files or code changes. @amotl I understand! I'll refrain from pointing out formatting shortcomings in ✏️ Learnings added
|
README.md
Outdated
#### VS Code Copilot Chat | ||
|
||
Please explore other [example questions] from a shared collection. | ||
Add the following configuration to your VS Code settings: | ||
```json | ||
{ | ||
"mcp": { | ||
"servers": { | ||
"cratedb-mcp": { | ||
"command": "uvx", | ||
"args": ["cratedb-mcp", "serve"], | ||
"env": { | ||
"CRATEDB_CLUSTER_URL": "http://localhost:4200/", | ||
"CRATEDB_MCP_TRANSPORT": "stdio" | ||
} | ||
} | ||
} | ||
}, | ||
"chat.mcp.enabled": true | ||
} | ||
``` |
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.
This configuration snippet, picked from another documentation page, may possibly refer to a global configuration of VS Code?
The documentation page about how to Use MCP servers in VS Code refers to a more specific .vscode/mcp.json
configuration file instead.
It makes sense, because the example configuration snippet on this page omits the parent mcp
element, directly referring to servers
instead.
Let's also follow this approach? NB: Please provide advises in retrospective if the patch may get merged early.
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.
We clarified the difference between user-wide vs. per-workspace configuration with a81e24a, and also mentioned the automatic discovery option.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_examples.py (1)
9-12
: Test looks good, but consider enhancing assertionsThe test correctly verifies that the script runs successfully and outputs "Ready." However, for more robust testing, consider adding additional assertions that verify other key aspects of the script's behavior.
- assert b"Ready." in proc.stdout + # Verify expected output contains key information + assert b"Ready." in proc.stdout + # Add more specific assertions about expected output patterns + # For example: + # assert b"Configuration loaded" in proc.stdout
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/tests.yml
(1 hunks)examples/mcptools.sh
(1 hunks)tests/test_examples.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/mcptools.sh
🔇 Additional comments (4)
.github/workflows/tests.yml (2)
71-74
: LGTM: Great addition for supporting the mcptools exampleThe new step for setting up mcptools aligns well with the PR objective of relocating mcptools examples into a dedicated script at
examples/mcptools.sh
. This ensures the CI environment can properly execute and validate the example.
71-74
:✅ Verification successful
Consider adding Go setup step if not already present
The workflow assumes Go is already installed on the runner. While Ubuntu runners typically have Go pre-installed, it might be worth explicitly adding a Go setup step to ensure compatibility if runner images change in the future.
🏁 Script executed:
#!/bin/bash # Check if Go is pre-installed on the GitHub runner go version || echo "Go is not installed on the runner"Length of output: 145
No Go setup step needed on Ubuntu runners
Verified that Go 1.24.1 is already installed on the
ubuntu-latest
runner (output ofgo version
isgo1.24.1 linux/amd64
). An explicit Go install step is therefore not required to ensure Go is available.• If you’d like to pin a specific Go version for future stability, you can optionally add:
- uses: actions/setup-go@v4 with: go-version: 1.24
tests/test_examples.py (2)
1-6
: Good use of linter directives and required importsThe linter directives appropriately disable security warnings for subprocess usage, and all necessary imports are included.
8-8
:❓ Verification inconclusive
Verify the executable name in the skipif condition
The test skips if "mcpt" is not found, but the workflow installs "mcptools". If these are different executables, consider aligning the test condition with what's actually installed in CI.
🏁 Script executed:
#!/bin/bash # Check what executables are installed by the mcptools package which mcptools which mcpt ls -la $(which mcptools 2>/dev/null || echo "/nonexistent") $(which mcpt 2>/dev/null || echo "/nonexistent")Length of output: 450
🏁 Script executed:
#!/bin/bash # Search for CLI entry points to determine the actual installed executable name grep -R "console_scripts" -n . grep -R "entry_points" -n setup.py pyproject.tomlLength of output: 122
Verify external CLI executable name
The skip condition in
tests/test_examples.py
checks formcpt
, but the CI installs themcptools
package. Please confirm whether the installed executable is namedmcpt
ormcptools
. If the tool’s CLI is actuallymcptools
, update the skipif to:- @pytest.mark.skipif(not which("mcpt"), reason="requires mcptools") + @pytest.mark.skipif(not which("mcptools"), reason="requires mcptools")tests/test_examples.py:8
The `uvx` launcher command is provided by the [uv] package manager. | ||
The [installation docs](#install) section includes guidelines on how to | ||
install it on your machine. |
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.
Excerpt from the backlog, about getting rid of uvx
?
Lines 14 to 16 in 89a4dc0
- Install: Others also just use `docker` right away? Others discourage using `uvx`. | |
- https://github.com/ppl-ai/modelcontextprotocol/?tab=readme-ov-file#step-3-configure-claude-desktop | |
- https://medium.com/@scalablecto/stop-running-your-mcp-tools-via-npx-uvx-right-now-380d1ab99d3f |
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.
Looks like consensus hasn't been reach yet. Let's return to it once it has
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.
I took a note about it.
The `uvx` launcher command is provided by the [uv] package manager. | ||
The [installation docs](#install) section includes guidelines on how to | ||
install it on your machine. |
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.
Looks like consensus hasn't been reach yet. Let's return to it once it has
About
A bit of copy-editing to support the upcoming v0.0.1 release.
Details
To better support the most prominent audience of users, let's provide configuration snippets for the most common AI assistant applications closer to the top of the README document, within the new "Quickstart Guide" section.
To unclutter the bottom of the page, the dry-dock examples using the mcptools swiss army knife tool to invoke MCP tools of the CrateDB MCP Server from the command line have been refactored into a dedicated example program at
examples/mcptools.sh
.Outlook
Any patches of the same nature are welcome to improve guidance matters. At some point, when details will grow out the single README page, we will need to break out information into individual pages.