-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Moved issue tests to subdirs and normalised names #59120
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
Conversation
The reorganization here makes sense so I'd r+ myself but I'll let someone from T-compiler OK this... r? @varkor |
☔ The latest upstream changes (presumably #59044) made this pull request unmergeable. Please resolve the merge conflicts. |
r=me after rebasing |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The The end goal for the |
The only thing I'd add to what @petrochenkov said is that having |
This comment has been minimized.
This comment has been minimized.
Sounds fair enough. Since that may not happen for a while though, and will take a fair bit of effort, I think this is a good stop-gap solution. |
A stop-gap solution shouldn't go into the opposite direction from the end goal, IMO. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I also don't think this is going in the right direction. There's nothing per se that relates any of the |
I do think this is an improvement and I do think it advances @petrochenkov's goal (which I agree with) of:
Specifically, what I like about this PR is not consistency but that it reduces the amount of files that are directly in |
@Centril Yeah, consistency was only one goal: decluttering the directories like |
cc @QuietMisdreavus for the Other changes are unnecessary churn, IMO. Other test suites are already small (so the GitHub display problem is not relevant for them), and tests from |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@bors r=estebank p=1 |
📌 Commit 44658ad3b75c63ab9fbcade376b4c783d4bb8d80 has been approved by |
⌛ Testing commit 44658ad3b75c63ab9fbcade376b4c783d4bb8d80 with merge de1bbdb996703d26d6ccdc3f85b74bc48f4dd768... |
Huh? I'd been given the go-ahead to do r=estebank. I admittedly should have waited until CI passed, but I had to leave, and tests were passing locally, so... |
The point was that since the review approval was given, additional points of discussion have been raised for which there's not a clear consensus yet, and triggering bors is effectively choosing to ignore that. |
This comment has been minimized.
This comment has been minimized.
@alexreg |
@varkor r+? |
@bors r+ |
📌 Commit 84fc24fa14961a05ddf3a2279e53f1e988e4c754 has been approved by |
⌛ Testing commit 84fc24fa14961a05ddf3a2279e53f1e988e4c754 with merge b4f102e6dfed6d6d015b63c7c867b303bb8c0bed... |
💔 Test failed - status-appveyor |
|
📌 Commit fe30743 has been approved by |
Moved issue tests to subdirs and normalised names Consistency, decluttering, ease of navigation :-) r? @Centril
☀️ Test successful - checks-travis, status-appveyor |
Consistency, decluttering, ease of navigation :-)
r? @Centril