Skip to content

Conversation

albertlarsan68
Copy link
Member

@albertlarsan68 albertlarsan68 commented Jan 18, 2023

@ozkanonur if you want to be assigned to review PRs too, just post a message to this thread.

Should a T-bootstrap label be created, since src/tools/tidy is assigned to the bootstrap members, but labeled A-testsuite (and not A-bootstrap) ?

cc @jyn514

@rustbot
Copy link
Collaborator

rustbot commented Jan 18, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 18, 2023
@onur-ozkan
Copy link
Contributor

if you want to be assigned to review PRs too, just post a message to this thread.

Yes, I would like to.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Should a T-bootstrap label be created, since src/tools/tidy is assigned to the bootstrap members, but labeled A-testsuite (and not A-bootstrap) ?

I think renaming A-bootstrap to T-bootstrap is fine, but I don't think we need a special label just to distinguish tidy - if you're not comfortable reviewing changes to it, you can assign Mark directly without assigning the whole bootstrap team.

@albertlarsan68
Copy link
Member Author

I meant that now, the tidy tool is assigned to the bootstrap ad-hoc team, but currently gets the A-testsuite label. It is the case with others parts, I gave tidy as an example.

@jyn514
Copy link
Member

jyn514 commented Jan 18, 2023

My question to you is: are those tools you're interested in working on? :)

Bootstrap does not have a formal mandate and "build system" is quite broad. If you want to be involved in those tools, I think labeling them as bootstrap is totally fine. If you don't want to be involved in those tools, I think removing the bootstrap team from the list of reviewers is also totally fine.

I would be curious to hear if @Mark-Simulacrum has opinions on whether tidy/compiletest/rust-installer should be under T-bootstrap's mandate or not.

@albertlarsan68
Copy link
Member Author

I would move tidy to the T-bootstrap team, and leave the rest in the A-testsuite label, maybe additionally adding the T-bootstrap label.

If I read triagebot.toml correctly, those paths are assigned to bootstrap:

  • src/bootstrap
  • src/stage0.json: This is labeled to T-release and A-bootstrap
  • src/tools/compiletest: Labeled as A-testsuite
  • src/tools/rust-installer
  • src/tools/tidy: Labeled as A-testsuite
  • src/tools/x

Some of the tools labeled as A-testsuite do not have assign directives.

@jyn514
Copy link
Member

jyn514 commented Jan 18, 2023

That sounds reasonable to me :)

@albertlarsan68
Copy link
Member Author

What is the process for the rename of the A-bootstrap label to T-bootstrap?

@jyn514
Copy link
Member

jyn514 commented Jan 18, 2023

Done just now :) (if you need to do this yourself, I went to https://github.com/rust-lang/rust/labels?q=bootstrap in the web interface and clicked the edit button)

@albertlarsan68 albertlarsan68 force-pushed the patch-2 branch 3 times, most recently from 51952b0 to e17d413 Compare January 18, 2023 15:27
@albertlarsan68
Copy link
Member Author

albertlarsan68 commented Jan 18, 2023

I would have done that once the autolabel is changed, but I do not have write rights to the rust repo.

@ozkanonur and @Mark-Simulacrum do you have thoughts ?

@onur-ozkan
Copy link
Contributor

src/tools/tidy is assigned to the bootstrap members, but labeled A-testsuite (and not A-bootstrap) ?

I think tidy should either not assign bootstrap members or be labeled as A-bootstrap. Because right now it seems confusing to me :)

p.s: rustbot gets confused by A-bootstrap currently.
ref #106621 (comment)

@albertlarsan68
Copy link
Member Author

I reverted the tidy label move.
@ozkanonur, if you don't feel like reviewing tidy changes, feel free to reassign it to another member of the team.

@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jan 21, 2023

📌 Commit fa7d17d has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@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 Jan 21, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 21, 2023
…ulacrum

Add new bootstrap members to triagebot.toml

`@ozkanonur` if you want to be assigned to review PRs too, just post a message to this thread.

Should a `T-bootstrap` label be created, since `src/tools/tidy` is assigned to the `bootstrap` members, but labeled `A-testsuite` (and not `A-bootstrap`) ?

cc `@jyn514`
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jan 22, 2023
…ulacrum

Add new bootstrap members to triagebot.toml

``@ozkanonur`` if you want to be assigned to review PRs too, just post a message to this thread.

Should a `T-bootstrap` label be created, since `src/tools/tidy` is assigned to the `bootstrap` members, but labeled `A-testsuite` (and not `A-bootstrap`) ?

cc ``@jyn514``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 22, 2023
…mpiler-errors

Rollup of 8 pull requests

Successful merges:

 - rust-lang#103418 (Add `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` to future-incompat report)
 - rust-lang#106113 (llvm-wrapper: adapt for LLVM API change)
 - rust-lang#106144 (Improve the documentation of `black_box`)
 - rust-lang#106578 (Label closure captures/generator locals that make opaque types recursive)
 - rust-lang#106749 (Update cc to 1.0.77)
 - rust-lang#106935 (Fix `SingleUseLifetime` ICE)
 - rust-lang#107015 (Re-enable building rust-analyzer on riscv64)
 - rust-lang#107029 (Add new bootstrap members to triagebot.toml)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 31f9f21 into rust-lang:master Jan 22, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 22, 2023
@jyn514
Copy link
Member

jyn514 commented Jan 22, 2023

I've updated the A-bootstrap label back to T-bootstrap :)

@albertlarsan68 albertlarsan68 deleted the patch-2 branch January 23, 2023 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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