Conversation
rust-highfive
added
the
S-waiting-on-review
ollie27
left a comment
ollie27
left a comment
There was a problem hiding this comment.
Could we combine these three files into a single tester.js file that works for both test suites?
| return null; | ||
| } | ||
| // We make a reverse sort to have the "highest" file. Very useful in case you didn't clean up | ||
| // you std doc folder... |
There was a problem hiding this comment.
This is a lot of code for such an edge case.
Instead, how about passing the resource suffix as an argument to tester.js and using that to build the filenames? That way we won't need the findFile or readFileMatching functions at all and there won't be any issues with outdated files.
There was a problem hiding this comment.
That'd make things simpler indeed!
|
@ollie27 I'm a bit shared on this: it'd mean adding more options, which I'm not a big fan of. |
I don't think merging the files would, but either way it can done in a follow up PR. |
joelpalmer
added
S-waiting-on-review
Dylan-DPC-zz
added
S-waiting-on-author
|
@bors r+ rollup |
|
📌 Commit c88be65 has been approved by |
bors
added
S-waiting-on-bors
|
Ok, opening an issue to not lose track of it then. Thanks a lot @ollie27 ! |
|
#70224 (comment) still hasn't been resolved. I guess I should have been clearer but that was the reason I hadn't r+'d this. |
Rollup of 5 pull requests Successful merges: - rust-lang#68334 (AArch64 bare-metal targets: Build rust-std) - rust-lang#70224 (Clean up rustdoc js testers) - rust-lang#70532 (Miri engine: stronger type-based sanity check for assignments) - rust-lang#70698 (bootstrap: add `--json-output` for rust-analyzer) - rust-lang#70715 (Fix typo in operands section) Failed merges: r? @ghost
6 participants
I realized after the improvement made by @ollie27 on the rustdoc-js-tester that a lot of code was actually duplicated. This PR intends to remove this duplication, making it simpler to update in case of future main.js updates.
r? @ollie27
cc @kinnison