Skip to content
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

Include uv in cli benchmarks #1771

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dimbleby
Copy link

No description provided.

Copy link
Collaborator

@ofek ofek left a comment

Choose a reason for hiding this comment

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

Thanks! I like the --shell=none change but I think including UV isn't meaningful because it's not in Python i.e. you might as well plug in any other CLI of a compiled language like git. Could you please remove that part and just keep the former modification?

@dimbleby
Copy link
Author

dimbleby commented Oct 26, 2024

I included the --shell=none only because hyperfine otherwise complained that shell startup was too significant in the uv benchmarking, it is otherwise pretty irrelevant.

I do not agree that comparison is only valid if the other tool is written in python. Why would users care about that? If this is measuring something useful - then surely it is just as useful for uv as for anything else?

uv is a relevant comparison where eg git is not because it is an "equivalent" (ish!) tool: just as poetry and pipenv are relevant comparisons but, say, black and ansible would not be.

Of course this benchmark will show that uv is fast, which perhaps is not the point you want to make - but that is how things are!

@ofek
Copy link
Collaborator

ofek commented Oct 26, 2024

I would prefer the benchmark job be removed then, and the reference in the documentation.

@dimbleby
Copy link
Author

sure, #1772

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