-
Notifications
You must be signed in to change notification settings - Fork 148
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
Add command for downloading a new benchmark #1183
Conversation
44eefc0
to
e989dbc
Compare
Is it necessary to be able to get the crate by cloning from github? That's what I was doing and this morning @lqd explained how to download from crates.io and my experience so far is that the crates.io way is simpler and less error-prone. |
Sure, but I suppose that not all crates are on crates.io? For example people that create performance regression issues on the |
e989dbc
to
e5824e4
Compare
I changed the logic to guess the unpacked package name and added a comment. |
Yeah, I agree that creating that file would be good. I'm not sure why CI is failing, but that seems like the main blocker left here. |
@nnethercote You are completely right :) I just didn't add it here since I wasn't sure which PR would get merged first. CI seems broken since some package is missing, I'm not sure if its transient or not 🤷 Maybe the workflow will need updating. |
The package still returns 404, so we might need to update package versions on CI. |
Rebased over master with fixed CI. |
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 good, just a couple of minor suggestions.
Inspired by Zulip conversion between @nnethercote and @Mark-Simulacrum.
Usage: