Fix missing minification for static files #104404
Conversation
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
|
@bors rollup |
…iaskrgr Rollup of 13 pull requests Successful merges: - rust-lang#103842 (Adding Fuchsia compiler testing script, docs) - rust-lang#104354 (Remove leading newlines from `NonZero*` doc examples) - rust-lang#104372 (Update compiler-builtins) - rust-lang#104380 (rustdoc: remove unused CSS `code { opacity: 1 }`) - rust-lang#104381 (Remove dead NoneError diagnostic handling) - rust-lang#104383 (Remove unused symbols and diagnostic items) - rust-lang#104391 (Deriving cleanups) - rust-lang#104403 (Specify language of code comment to generate document) - rust-lang#104404 (Fix missing minification for static files) - rust-lang#104413 ([llvm-wrapper] adapt for LLVM API change) - rust-lang#104415 (rustdoc: fix corner case in search keyboard commands) - rust-lang#104422 (Fix suggest associated call syntax) - rust-lang#104426 (Add test for rust-lang#102154) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
This caused a significant |
|
It was not expected. Minification isn't supposed to take so much time. There isn't much we can do about it though I'm afraid... Maybe check if the file exists before generating it? I'll send a PR for that. |
|
Actually I need the file to be minified to get the hash so it wouldn't change anything. I guess there is nothing we can do in here. It's just a perf regression that wasn't taken into account when #101702 was made because it wasn't using minification. |
|
Actually, #101702 had a performance improvement of the same size as the regression. At the time I thought it was because it was more efficiently interpolating filenames; but in retrospect it looks like the improvement was due to accidentally not minifying anything. So I think the perf regressions from this PR just take us back to baseline. |
|
More or less yes. I personally think it's acceptable for the time being to not require an immediate action. We are working on performance continuously so it'll be fixed at some point. :) |
It's a fix for #101702.
The problem was that
Path::ends_withdoesn't do what we thought it does: it checks if the entire item is the last path part, no just if the "path string" ends with the given argument. So instead, I just used theextension()method to get the information we want.cc @jsha
r? @notriddle
PS: Is it worth it to add a CI test to ensure that the minification was performed on JS and CSS files or not?