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

Require (and potentially enforce) attribute-style // issue: rust-lang/rust#12345 comments in issue-related tests #113604

Open
tgross35 opened this issue Jul 12, 2023 · 4 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@tgross35
Copy link
Contributor

tgross35 commented Jul 12, 2023

This comes as a followup to #113583

The goal is to add meta comments to all tests related to an issue or ICE (e.g. regression tests) so that we can programmatically extract this information. For example:

// issue: rust-lang/rust#12345
// issue: rust-lang/rust#6789
// ice: rust-lang/rust#1234

// sample link to different repo:
// issue: rust-lang/cargo:1234   

We should make having this a goal for all regression tests going forward, and as a requirement with any renaming that might happen to suit #113583. We can likely do some of this programmatically.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 12, 2023
@tgross35
Copy link
Contributor Author

@rustbot label +A-testsuite +T-bootstrap -needs-triage

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jul 12, 2023
@workingjubilee
Copy link
Member

workingjubilee commented Jul 12, 2023

We should probably allow naming issues that aren't in the main repo, so it should probably be more like {repo}#{issue}.

EDIT: Please disregard my inability to read more than 5 lines.
EDIT EDIT: The "real" reason to use repo#issue is that {org}/{repo}#issue is a shorthand in particular that GitHub understands and will "linkify" while browsing commit messages.

@matthiaskrgr
Copy link
Member

Yeah we need a clear naming scheme for subtrees, for example when you add a test to clippy, inside the clippy repo, issue:123 looks fine because of course it refers to the clippy repo. But then it gets subtree-merged into the rustc repo and would point at some random rust-lang/rust ticket from 7 years ago (because the test is somewhere in src/tools/clippy inside the rustc repo now?)

@tgross35 tgross35 changed the title Require (and potentially enforce) attribute-style // issue:12345 comments in issue-related tests Require (and potentially enforce) attribute-style // issue: rust-lang/rust#12345 comments in issue-related tests Jul 13, 2024
@notriddle
Copy link
Contributor

I've been using URLs because my editor lets me Ctrl+Click a URL to open it in a browser, while shorthand $org/$repo#$number only works in GitHub itself. Not sure if that's a big enough deal to justify using full URLs, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

No branches or pull requests

5 participants