Uh oh!
There was an error while loading. Please reload this page.
Trim runtest - #89240
Conversation
rust-highfive
commented
Sep 25, 2021
(rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
jyn514
commented
Sep 25, 2021
Why this change? Rustc will happily accept |
Nicholas-Baron
commented
Sep 25, 2021
@jyn514 More a stylistic preference than anything else. I will be redoing this PR as some smaller changes were unnecessary. |
jyn514
commented
Sep 25, 2021
@Nicholas-Baron it seems better to keep the original filename to avoid more churn than necessary. |
4469251 to
e885baeComparebors
commented
Sep 26, 2021
☔ The latest upstream changes (presumably #89101) made this pull request unmergeable. Please resolve the merge conflicts. |
camsteffen
commented
Sep 27, 2021
Rather than |
Nicholas-Baron
commented
Sep 30, 2021
@camsteffen While I agree with the sentiment, the |
e885bae to
8133f4dCompareMark-Simulacrum
commented
Oct 2, 2021
I think this is not quite the change I would expect for this goal. Generally speaking, just moving code to a separate file (particularly when the move is based on a somewhat arbitrary line) isn't really making it easier to understand the file I think. Instead, I would suggest some of these changes as a start, if you'd like to cleanup compiletest a little. I think these will lead to slightly better structure for the code, while keeping the bits that have a shared structure to them in the same place for easier reading.
I suspect this set of changes is likely harder, and might not get to <3000 lines; there's likely more cleanup work possible though. This module has grown pretty organically over the last ~10 years -- there's a lot of small bits that can be adjusted to make things better. There's probably also subtle inconsistencies between various functions that would likely be nice to cleanup and unify, and filing PRs for those would be helpful -- likely some care is needed, perhaps even some small test updates, but overall a good goal. You might also be interested in #40713, as a somewhat larger project. |
Nicholas-Baron
commented
Oct 2, 2021
@Mark-Simulacrum Thank you for the suggestions. I have spun them off as a "subtask", that will probably take multiple PRs to finish. |
Work towards #60302.
src/tools/compiletest/src/runtest.rswas over 4k lines. To break it down into smaller chunks,TestCxstruct was extracted, as it constituted the bulk of the file.run_foo_testwere moved into a separate file, as they were ~1k lines by themselves.