Skip to content

Conversation

@Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Oct 29, 2025

I found myself confused by these percentage deltas, because it's not immediately obvious whether a change of 110% means “a little bit worse” (+10%, 110% overall) or ”more than twice as long” (+110%, 210% overall). The correct interpretation is “more than twice as long”.

Including a sign for positive percentages should hopefully make it clearer that they are relative to a baseline of ±0%, not a baseline of 100%.

Manually tested with:

cargo run --manifest-path src/ci/citool/Cargo.toml post-merge-report df984edf44203c862e01b5a20c8092d5614d872e c9537a94a6300a8292804829801f7724fb8a33f6

r? Kobzol

@rustbot rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Oct 29, 2025
@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

Makes sense, thanks!

@bors r+ rollu

@bors
Copy link
Collaborator

bors commented Oct 29, 2025

📌 Commit 51f3cab has been approved by Kobzol

It is now in the queue for this repository.

@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

@bors rollup

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2025
bors added a commit that referenced this pull request Oct 29, 2025
Rollup of 4 pull requests

Successful merges:

 - #148159 (Move rustdoc tests to appropriate subdirectories)
 - #148218 (Update T-compiler/ops Zulip messages)
 - #148228 (Run regression test for #147964 on next solver too)
 - #148237 (citool: Always print a signed percentage in job duration changes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e677944 into rust-lang:master Oct 29, 2025
11 checks passed
rust-timer added a commit that referenced this pull request Oct 29, 2025
Rollup merge of #148237 - Zalathar:analysis-signed, r=Kobzol

citool: Always print a signed percentage in job duration changes

I found myself confused by these percentage deltas, because it's not immediately obvious whether a change of 110% means “a little bit worse” (+10%, 110% overall) or ”more than twice as long” (+110%, 210% overall). The correct interpretation is “more than twice as long”.

Including a sign for positive percentages should hopefully make it clearer that they are relative to a baseline of ±0%, not a baseline of 100%.

Manually tested with:
```text
cargo run --manifest-path src/ci/citool/Cargo.toml post-merge-report df984ed c9537a9
```

r? Kobzol
@rustbot rustbot added this to the 1.93.0 milestone Oct 29, 2025
@Zalathar Zalathar deleted the analysis-signed branch October 29, 2025 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants