-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Update bug.yaml w/ fully-self-contained client+server example #1432
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
base: main
Are you sure you want to change the base?
Conversation
#!/usr/bin/env uv run | ||
# /// script | ||
# requires-python = ">=3.11" | ||
# dependencies = [ | ||
# "anyio", | ||
# "mcp", | ||
# ] | ||
# /// |
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.
Does this do anything? It's not obvious to me whether this needs to be included in the file or not or create a pyproject.toml
file for this or something. Is this just a listing to explicitly list dependencies you might have to add?
If it does more than just list dependencies in plaintext it could be cool to have a comment like # list the dependencies needed to run your example here
or something.
Otherwise I think this is a 10x improvement over the nonexistent example right now.
Makes it much more obvious what a good code example is!
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.
Does this do anything?
uv run
installs what is in those headers. uv run --help | grep script
locally, and you can read about it.
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'm not entirely convinced we need this.
At least from most of the examples I've run into they've not needed third party dependencies, and I think it's reasonable to assume the maintainers will have mcp and anyio installed as it's the requirements of the repo.
Although it's supported by uv, it's a rather obscure part of uv and may confuse users.
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.
It's a python thing: https://peps.python.org/pep-0723/
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.
Oh huh I totally thought it was just uv.
I'm not fully against adding it if others agree, but I'm not convinced we need it.
I would find it annoying to do it. 🤷♂️ |
Which part? Asking people to provide code snippets, or is the one here too verbose or something? |
We chatted about updating the bug template to get users to provide full repro test cases.
This might be a bit too much but shows a single-file, no-dep (besides uv) client+server example as an easy starting point.
@felixweinberger @maxisbey @Kludex wdyt?
(alternatively, could create a lil markdown we'd point to instead of https://stackoverflow.com/help/minimal-reproducible-example, which would include this example and maybe more)