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

Outside of examples, fill in all edition attrs #1257

Merged
merged 1 commit into from Apr 25, 2022

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Apr 11, 2022

This is extracted from #1256 in case these changes are easier to review separately.

This is effectively a fully exhaustive version of #1254. Due to #1251, specifying edition in each target is required in order for downstream workspaces importing rules_rust to get a usable edition for all of rules_rust's targets, instead of attempting to use the downstream workspace's default edition which won't necessarily be appropriate.

Note that under examples, I have added edition to the rust_register_toolchains in various WORKSPACE.bazel files rather than to each individual target. This reduces noise in the example projects. I have not done the same for the top-level WORKSPACE because doing so would mask trouble along the lines of #1254.

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Thanks!

@UebelAndre UebelAndre requested a review from dfreese April 12, 2022 12:30
@UebelAndre
Copy link
Collaborator

@dfreese do you have time to take another look at these PRs?

@UebelAndre
Copy link
Collaborator

In the absence of @dfreese, my understanding is that this generally meets their concerns and I think this and the related changes are ready to merge. @dtolnay can you rebase?

@dtolnay dtolnay force-pushed the edition branch 2 times, most recently from 245db44 to 661d599 Compare April 20, 2022 17:32
@dtolnay
Copy link
Contributor Author

dtolnay commented Apr 20, 2022

@dtolnay
Copy link
Contributor Author

dtolnay commented Apr 24, 2022

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.

None yet

2 participants