Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use optflag for --report-time #93479

Merged
merged 1 commit into from
Feb 17, 2022
Merged

Use optflag for --report-time #93479

merged 1 commit into from
Feb 17, 2022

Conversation

smoelius
Copy link
Contributor

Essentially, what is described here:
#64888 (comment)

There is one difference. The comment proposes to add a
--report-time-color option. This change instead uses libtest's
existing --color option for that purpose.

Essentially, what is described here:
rust-lang#64888 (comment)

There is one difference. The comment proposes to add a
`--report-time-color` option. This change instead uses libtest's
existing `--color` option for that purpose.
@rust-highfive
Copy link
Collaborator

r? @yaahc

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 30, 2022
Copy link
Contributor

@popzxc popzxc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smoelius
Copy link
Contributor Author

@yaahc Is there anything I can do to make this PR easier to review? (Sorry to be a nuisance.)

@yaahc
Copy link
Member

yaahc commented Feb 14, 2022

@yaahc Is there anything I can do to make this PR easier to review? (Sorry to be a nuisance.)

No, I expect this PR is fine as is, and no worries you're not being a nuisance, I understand it can be frustrating to have these PRs sit around for a long time, especially when they seem relatively simple. I'm sorry for not getting to this sooner but I have a limited amount of time to dedicate to reviews and have to prioritize which PRs I review first, and libtest is sadly pretty low on that priority list right now. For insight I'm currently reviewing #90822 which is time consuming because I need to increase my familiarity with the Allocator APIs and read a bunch (years) of historical discussion. Also here are the rest of the PRs I have on my todolist: https://github.com/rust-lang/rust/pulls?q=is%3Aopen+is%3Apr+assignee%3Ayaahc+label%3AS-waiting-on-review+sort%3Acreated-asc, as you can see some have been waiting for quite a while 😅 . I'll get to this as soon as I can, and thank you for your patience.

@smoelius
Copy link
Contributor Author

I'm sorry, I should have kept my mouth shut. Good luck with #90822 and the rest of the PRs on your list. 🙌 There is no rush.

@yaahc
Copy link
Member

yaahc commented Feb 14, 2022

I'm sorry, I should have kept my mouth shut.

nonono, please, an apology isn't necessary and wasn't my goal with the response. Things can and do fall through the cracks and these pings can be useful, its just a difficult balance. You did nothing wrong and in similar PRs in the future I'd prefer if you feel comfortable to reach out if you think there may be something wrong.

@yaahc
Copy link
Member

yaahc commented Feb 16, 2022

Looks great, thank you again for the PR!

@bors r+

@bors
Copy link
Contributor

bors commented Feb 16, 2022

📌 Commit 96d96a7 has been approved by yaahc

@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 Feb 16, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 16, 2022
Use `optflag` for `--report-time`

Essentially, what is described here:
rust-lang#64888 (comment)

There is one difference. The comment proposes to add a
`--report-time-color` option. This change instead uses libtest's
existing `--color` option for that purpose.
@smoelius
Copy link
Contributor Author

Looks great, thank you again for the PR!

Thank you!

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 17, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#89869 (Add documentation to more `From::from` implementations.)
 - rust-lang#93479 (Use `optflag` for `--report-time`)
 - rust-lang#93693 (Suggest deriving required supertraits)
 - rust-lang#93981 (Fix suggestion to slice if scurtinee is a reference to `Result` or `Option`)
 - rust-lang#93996 (Do not suggest "is a function" for free variables)
 - rust-lang#94030 (Correctly mark the span of captured arguments in `format_args!()`)
 - rust-lang#94031 ([diagnostics] Add mentions to `Copy` types being valid for `union` fields)
 - rust-lang#94064 (Update dist-x86_64-musl to Ubuntu 20.04)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d855121 into rust-lang:master Feb 17, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants