-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Progress bars for uv publish
#7613
Conversation
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.
LGTM, although I think I would probably try to avoid defining a new trait here in favor of a concrete type or enum. (But I may be missing some context motivating the trait.)
fn on_download_start(&self, name: &str, size: Option<u64>) -> usize; | ||
fn on_download_progress(&self, id: usize, inc: u64); | ||
fn on_download_complete(&self); | ||
} |
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.
How come you went for a trait here instead of asking for a concrete type? An enum would probably do the job in order to handle the case of the "dummy" reporter.
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 think this matches our other reporter patterns?
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.
Ah. That would be a candidate for un-genericizing code in a hope to improve compile times. :)
(It may result in little gain if there's only one instantiation of the generics outside of tests though.)
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 would be nice to have a single reporter crate that is shared (or some callback) instead of copying this code for every new feature.
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.
Yeah. I think it can be completely monomorphic too with no parametric polymorphism.
33562d6
to
8ea5a02
Compare
fdce2be
to
1fdc856
Compare
1fdc856
to
ace6bae
Compare
e4a765f
to
7eed9bd
Compare
ace6bae
to
e4c838b
Compare
Add progress bars to
uv publish
, helpful when uploading large packages.