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

Change handling of renames #1549

Merged
merged 8 commits into from
Nov 27, 2018
Merged

Change handling of renames #1549

merged 8 commits into from
Nov 27, 2018

Conversation

nrc
Copy link
Member

@nrc nrc commented Nov 21, 2018

This PR makes handling of renamed components more flexible by allowing components to be added or removed based on either the 'before' or 'after' names. We always use the 'before' name when communicating with the user, but use the 'after' name internally (basically just in the manifest).

This is a backwards-compatible stop-gap. It means that for the edition we can talk about 'rls' rather than 'rls-preview'. (Assuming renames are from 'rls' to 'rls-preview', which is the current situation in the manifest).

The final commit adds an instruction for adding missing components to rustup's proxies.

Since Rustup is not distributed with Rust, this does not need to be part of the edition, timing-wise. However, it would be good to have this in the wild and tested somewhat so that new users will get the benefits when the edition is released.

r? @alexcrichton

@nrc
Copy link
Member Author

nrc commented Nov 21, 2018

Closes #1546

@alexcrichton
Copy link
Member

I'll be honest in that I don't really have a great handle on what's going on here, but this all looks largely like innocuous changes passing more parameters around and/or fallout from tweaking interfaces. In that sense the guts of the patch look good to me and I think we can probably go ahead and land.

I think there's some test failures on CI though? Otherwise r=me.

As one other thought, though, this is trying to use the "from" name in the renames to display names, right? Shouldn't it use the "to" names eventually? I think with this patch we'd start saying rustfmt, but then whenever we actually switch the package names it'll say rustfmt-preview, right?

@nrc
Copy link
Member Author

nrc commented Nov 21, 2018

Shouldn't it use the "to" names eventually?

We'll need more rustup changes once we want to switch again. We might just live with 'from' and 'to' being the wrong way around forever. But I don't think we can change this backwards compatibly.

@alexcrichton
Copy link
Member

Mk, I'm fine landing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants