Make must_not_suspend allow-by-default until edge cases are ironed out

[guswynn]

Fixes #89562

[rust-highfive]

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

[nagisa]

Please add a regression test for the problematic code as reported in the issue.

[nagisa]

Please also make the commit titles/descriptions more meaningful in isolation. Right now they don't contain any context indicating even what is being allowed by default. Copying the PR title to the commit message would help a lot here.

While at it, a squash would be good too.

r=me once history is rewritten.

[guswynn]

@nagisa my understanding was that bors merge made commit messages disappear, and that people typically review full pr's, not individual commits, is this understanding incorrect?

[nagisa]

my understanding was that bors merge made commit messages disappear

Bors merge is a standard git merge with a merge commit. As thus the history ends up containing both the commits that were a part of the PR at the time of merge and a merge commit. The merge commit will contain some information from PR, but not all of it is easily visible. In addition to this, in a timeline view the merge commit may be quite removed from the commits it merges, so you may end up with a listing that looks like this:

*   15491d7b6be - (origin/master, origin/HEAD) Auto merge of #89343 - Mark-Simulacrum:no-args-queries, r=cjgillot (7 days ago) <bors>
|\  
| * 415a9a2ea69 - (origin/pr/89343) Add a test that -Zquery-dep-graph -Zdump-dep-graph works (7 days ago) <Mark Rousskov>
| * 6f78eed1c75 - Query the fingerprint style during key reconstruction (10 days ago) <Mark Rousskov>
* |   bb918d0a5bf - Auto merge of #89698 - matthiaskrgr:rollup-gna54x6, r=matthiaskrgr (8 days ago) <bors>
|\ \  
| * \   2e5a5e22b21 - (origin/pr/89698) Rollup merge of #89697 - alessandrod:issue-89689, r=nikic (8 days ago) <Matthias Krüger>
| |\ \  
| | * | 8683d36042d - (origin/pr/89697) Fix min LLVM version for bpf-types test (8 days ago) <Alessandro Decina>
| * | |   827b540424c - Rollup merge of #89694 - jkugelman:must-use-string-transforms, r=joshtriplett (8 days ago) <Matthias Krüger>
| |\ \ \  
| | * | | 2ec7588aa1a - (origin/pr/89694) Update library/core/src/num/mod.rs (8 days ago) <John Kugelman>
| | * | | 54d807cfc7f - Add #[must_use] to string/char transformation methods (8 days ago) <John Kugelman>
| | |/ /  
| * | |   ee804594c83 - Rollup merge of #89693 - jkugelman:must-use-stdin-stdout-stderr-locks, r=joshtriplett (8 days ago) <Matthias Krüger>
| |\ \ \  
| | * | | e27bfb6e237 - (origin/pr/89693) Add #[must_use] to stdin/stdout/stderr locks (8 days ago) <John Kugelman>
| | |/ /  
| * | |   a06c664328c - Rollup merge of #89687 - Nicholas-Baron:move_read2_abbreviated, r=Mark-Simulacrum (8 days ago) <Matthias Krüger>
| |\ \ \  
| | * | | 8a4085d370a - (origin/pr/89687) Move read2_abbreviated function into read2.rs (8 days ago) <Nicholas-Baron>
| * | | |   0481c67dd46 - Rollup merge of #89684 - asquared31415:asm-doc-fix, r=joshtriplett (8 days ago) <Matthias Krüger>
| |\ \ \ \  
| | * | | | 4a565e51100 - (origin/pr/89684) Fix asm docs typo (8 days ago) <asquared31415>
| * | | | |   346f833c3dd - Rollup merge of #89678 - marcelo-gonzalez:master, r=joshtriplett (8 days ago) <Matthias Krüger>
| |\ \ \ \ \  
| | * | | | | 82c974dab5c - (origin/pr/89678) Fix minor std::thread documentation typo (8 days ago) <Marcelo Diop-Gonzalez>
| | |/ / / /  
| * | | | |   5ebb6a8fd93 - Rollup merge of #89641 - asquared31415:asm-feature-attr-regs, r=oli-obk (8 days ago) <Matthias Krüger>
| |\ \ \ \ \  
| | * | | | | 271da7d8bc9 - (origin/pr/89641) make #[target_feature] work with `asm` register classes (9 days ago) <asquared31415>
| * | | | | |   9d14b6505b3 - Rollup merge of #89634 - hawkw:eliza/enable-err-warn, r=oli-obk (8 days ago) <Matthias Krüger>
| |\ \ \ \ \ \  
| | * | | | | | 84fc5db59b3 - (origin/pr/89634) bless warnings (8 days ago) <Eliza Weisman>
| | * | | | | | 0e79545c306 - lol i forgot the syntax for my own crate's macros (9 days ago) <Eliza Weisman>
| | * | | | | | b6f09a19b23 - comma-related changes (9 days ago) <Eliza Weisman>
| | * | | | | | e00eac8b9c4 - use structured fields in some existing warnings (9 days ago) <Eliza Weisman>
| | * | | | | | 928c787fcee - make them structured while i'm here (9 days ago) <Eliza Weisman>
| | * | | | | | 01803025d2a - demote `rustc_peek` traces look not user-facing (9 days ago) <Eliza Weisman>
| | * | | | | | eb67bf93682 - Update compiler/rustc_driver/src/lib.rs (9 days ago) <Eliza Weisman>
| | * | | | | | e7f04857efc - rustc_driver: Enable the `WARN` log level by default (9 days ago) <Eliza Weisman>
| * | | | | | |   03a34a291d8 - Rollup merge of #89605 - camelid:fix-version, r=nagisa (8 days ago) <Matthias Krüger>
| |\ \ \ \ \ \ \  
| | * | | | | | | 6189d0a116c - (origin/pr/89605) Fix stabilization version for `bindings_after_at` (10 days ago) <Noah Lev>
| | | |/ / / / /  
| | |/| | | | |   
| * | | | | | | 36db6587965 - Rollup merge of #88707 - sylvestre:split_example, r=yaahc (8 days ago) <Matthias Krüger>
|/| | | | | | | 
| * | | | | | | d4031d092d0 - (origin/pr/88707) String.split_terminator: Add an example when using a slice of chars (6 weeks ago) <Sylvestre Ledru>
* | | | | | | |   910692de742 - Auto merge of #89582 - jkugelman:optimize-file-read-to-end, r=joshtriplett (8 days ago) <bors>

So unless the commits themselves are sufficiently descriptive, it can become quite difficult to scan the history when looking for something relevant.

people typically review full pr's, not individual commits, is this understanding incorrect?

This really depends on a situation as well as the reviewer. For smaller PRs, reviewing the entire PR as a whole is probably going to be common. When landing a PR requires more cycles of review feedback and fixes, or if the PR is fairly large reviewing commit-by-commit is going to be more common.

My concern here is more about the usability of the history once the PR is merged, and less so with the review flow, if that makes sense?

[guswynn]

@nagisa that makes sense, thanks for the thorough answer! I am quite new to git power-user workflows, is the best way to rename all the commits in a branch to use rebase -i with reword?

[nagisa]

git rebase -i is my go to approach to rewriting. squash or s command will combine the current commit into the previous one and give you an opportunity to adjust the commit message for the resulting commit at the same time. fixup will only combine and keep the message of the previous commit. And r will indeed allow you to change the commit message of the specified commit.

git commit --amend is great if you're just changing the wording of the most recent commit.

[mati865]

@guswynn could you address review feedback?

@rustbot modify labels: -S-waiting-on-review +S-waiting-on-author

[guswynn]

@mati865 I believe the only outstanding review was the commit names! Sorry about the delay, I was away for a few weeks

[camelid]

I think this PR should be backported to beta 1.57.0 too. Otherwise, after #89826 lands, there'd be no way to turn off the lint on 1.57.0 stable.

[camelid]

cc @Mark-Simulacrum in case you'd like to review this at the same time as #89826

[Mark-Simulacrum]

Pulled this PR into #89826, so closing this one. Thanks!

[apiraino]

beta nomination removed as per this comment