Skip to content

Conversation

@Urgau
Copy link
Member

@Urgau Urgau commented Jan 9, 2026

As we were reminded in #triagebot > Mentions glob matching, triagebot cannot see changes in submodules.

So let's reflect that in the tidy check to avoid accidentally adding paths inside submodules.

I tested it with these entries:

[mentions."src/tools/cargo"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/*"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/README.md"]
cc = ["@ehuss"]

and got (as expected):

tidy [triagebot]: triagebot.toml [mentions.*] 'src/tools/cargo/README.md' cannot match inside a submodule
tidy [triagebot]: triagebot.toml [mentions.*] contains 'src/tools/cargo/*' which doesn't match any file or directory in the repository

r? @tgross35

@rustbot rustbot added A-tidy Area: The tidy tool S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jan 9, 2026
// to be checked against all the file and directories in the repository.
builder.add(globset::Glob::new(&format!("{clean_path}*")).unwrap());
let trimmed_path = clean_path.trim_end_matches('/');
builder.add(globset::Glob::new(&format!("{trimmed_path}{{,/*}}")).unwrap());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be /,*?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's meant to be {,/*}.

We either want src/tools/cargo to be matched by it-self or anything under it (so src/tools/cargo/*).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. Compared to the previous version I think we lose src/tools/cargo-foo, is that important?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's what we want, we don't want people to be pinged for src/tools/cargotest if the mentions is for src/tools/cargo.

If they really want to be pinged for both, they can do src/tools/cargo* or src/tools/cargo{,test}.

(cf. #triagebot > ✔ Mentions glob matching @ 💬)

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks for explaining, LGTM

View changes since this review

@Urgau
Copy link
Member Author

Urgau commented Jan 9, 2026

@bors r=tgross35 rollup

@rust-bors rust-bors bot 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 9, 2026
@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 9, 2026

📌 Commit 9d2ce87 has been approved by tgross35

It is now in the queue for this repository.

rust-bors bot added a commit that referenced this pull request Jan 10, 2026
Rollup of 9 pull requests

Successful merges:

 - #149318 (Implement partial_sort_unstable for slice)
 - #150805 (Fix ICE in inline always warning emission.)
 - #150822 (Fix for ICE: eii: fn / macro rules None in find_attr())
 - #150853 (std: sys: fs: uefi: Implement File::read)
 - #150855 (std: sys: fs: uefi: Implement File::tell)
 - #150881 (Fix std::fs::copy on WASI by setting proper OpenOptions flags)
 - #150891 (Fix a trivial typo in def_id.rs)
 - #150892 (Don't check `[mentions]` paths in submodules from tidy)
 - #150894 (cg_llvm: add a pause to make comment less confusing)

r? @ghost
@rust-bors rust-bors bot merged commit 90de6e5 into rust-lang:main Jan 10, 2026
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Jan 10, 2026
rust-timer added a commit that referenced this pull request Jan 10, 2026
Rollup merge of #150892 - tidy-triagebot-mentions-submodules, r=tgross35

Don't check `[mentions]` paths in submodules from tidy

As we were reminded in [#triagebot > Mentions glob matching](https://rust-lang.zulipchat.com/#narrow/channel/224082-triagebot/topic/Mentions.20glob.20matching/with/567093226), triagebot cannot see changes in submodules.

So let's reflect that in the `tidy` check to avoid accidentally adding paths inside submodules.

I tested it with these entries:

```toml
[mentions."src/tools/cargo"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/*"]
cc = ["@ehuss"]
[mentions."src/tools/cargo/README.md"]
cc = ["@ehuss"]
```

and got (as expected):

```
tidy [triagebot]: triagebot.toml [mentions.*] 'src/tools/cargo/README.md' cannot match inside a submodule
tidy [triagebot]: triagebot.toml [mentions.*] contains 'src/tools/cargo/*' which doesn't match any file or directory in the repository
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-tidy Area: The tidy tool S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants