-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Make all download functions need only Config, not Builder #104188
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
@bors p=5 prone to bitrot |
In retrospect I probably should have separated the move to a different file into its own commit :( let me know if you want me to spend time on that. |
☔ The latest upstream changes (presumably #104078) made this pull request unmergeable. Please resolve the merge conflicts. |
Apologies for a slower review here -- r=me on this with a rebase. |
This also adds a new `mod download` instead of scattering the download code across `config.rs` and `native.rs`.
51de238
to
fb471de
Compare
Thanks! @bors r=Mark-Simulacrum rollup=iffy |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f90a4ff): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
This also adds a new
mod download
instead of scattering the download code acrossconfig.rs
andnative.rs
.This is the simplest and also most bit-rotty part of #102282. Opening it earlier so it's not mixed in with behavior changes and to avoid rebase hell.
cc #94829 (which nows has the hackmd linked).
r? @Mark-Simulacrum