-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix a test. #3558
Fix a test. #3558
Conversation
Presumably, it should failed when it was first written, but Cargo does not do such validation yet.
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
I am in general a bit worried that workspaces logic got changed in the ad hoc way form what was proposed in the RFC. I think that current variant is much better, but I am uneasy because it wasn't the result of an RFC process, so not so many people have actually scrutinized it. Perhaps this can be discussed at the tools team meeting, just to make sure we don't code ourselves in backwards compatibility corner? I've written a summary of what I think the rules should be here: #3443 (comment) |
@bors: r+ |
📌 Commit 2a9eacb has been approved by |
Fix a test. That's the test we've discussed in #3443
@matklad yeah I wouldn't want to tweak the logic without garnering more opinions. Do you want to propose that feature in a separate PR so we can discuss there? (sorry if this is just a runaround on various PRs...) |
☀️ Test successful - status-appveyor, status-travis |
#3562 And I have actually realized that one half of the implementation is totally missing from the PR :) I wonder if this means that we miss some validation? |
That's the test we've discussed in #3443