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

Convert all code to Rust 2018 edition #1508

Merged
merged 4 commits into from
Mar 31, 2021
Merged

Conversation

Rua
Copy link
Contributor

@Rua Rua commented Mar 14, 2021

  • Added an entry to CHANGELOG_VULKANO.md or CHANGELOG_VK_SYS.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes - in this repository
  • Updated documentation to reflect any user-facing changes - PR to the guide that fixes existing documentation invalidated by this PR.
  • Ran cargo fmt on the changes

This converts the whole codebase to use the 2018 edition of Rust. This mostly involves how use statements work. I've also fixed some of the less pedantic performance and style issues that clippy pointed out. There should be no functional changes, only in the code.

@Eliah-Lakhin
Copy link
Contributor

@Rua Is it a prerequirement for some major work?

@Rua
Copy link
Contributor Author

Rua commented Mar 15, 2021

No not really. I came across this while experimenting with Ash, and then decided to make a PR for it. I was kinda surprised Vulkano was still running on Rust 2015.

@Eliah-Lakhin
Copy link
Contributor

Eliah-Lakhin commented Mar 15, 2021

@Rua Would you mind if we postpone with the major code style change across the codebase? I like most of what clippy is proposing, and I constantly see a lot of the code that needs to be fresh up too, but I just don't think this is a good moment for this update. We are still in the API functional prototyping phase, I think that it would be better to make cleanup right before v1.0. Otherwise we will have to keep making it from time to time and it would mess with other people work in progress etc. Normally, I think that the codestyle should be established through the process of wider community discussion and maybe even enforced by Continuous Integration checks. At this moment we only enforce rust-fmt basic format.

@Rua
Copy link
Contributor Author

Rua commented Mar 15, 2021

So just the edition change, no clippy? I can do that.

@Eliah-Lakhin
Copy link
Contributor

@Rua Yes, let's make edition change only for now. I hope there are no 3rd party projects relaying on the older edition. Thank you!

@Rua
Copy link
Contributor Author

Rua commented Mar 15, 2021

Ok, here's a cleaner version that only changes to 2018.

@Rua Rua changed the title Convert all code to Rust 2018 edition, some clippy fixes Convert all code to Rust 2018 edition Mar 15, 2021
@Eliah-Lakhin
Copy link
Contributor

@Rua Still a lot of changes, though.

How do you think would it be feasible to finish #1430 in upcoming 0.22.0 release(let say within this month)? I would prefer to finish #1430 first because there is a work in progress that was already split between PRs(starting from #1504), and then as a very least change include this PR's migration into release. The problem with managing of the codebase wide PRs such as this one is that it's difficult to manage for me of what and where was changed. I want to keep track at least until we finish the minor release.

@Rua
Copy link
Contributor Author

Rua commented Mar 15, 2021

I think I can do that, yes.

@Rua
Copy link
Contributor Author

Rua commented Mar 28, 2021

Now that the execute_commands PR has been merged, what are the plans for this one?

@Eliah-Lakhin
Copy link
Contributor

@Rua Then plan is to merge #1517 then I will ask you to review this PR again and update missing things regarding the recent merges to master, and then I will proceed with release 0.22.0. All other PRs will be held until the next release.

@Eliah-Lakhin
Copy link
Contributor

So many breaking changes in upcoming release... I think I will have to write migration instructions and pin it to Github Issues once we released.

@Eliah-Lakhin
Copy link
Contributor

@Rua Could you please verify this PR again to make sure it is covering the recent changes in master?

@Rua
Copy link
Contributor Author

Rua commented Mar 31, 2021

It looks good, ready for merging. :)

I would recommend also merging #1523 for the next version, it seems useful and doesn't break anything.

@Eliah-Lakhin Eliah-Lakhin merged commit 4f009c3 into vulkano-rs:master Mar 31, 2021
@Eliah-Lakhin
Copy link
Contributor

@Rua Merged 👍

I also merged #1523, but it resolves the issue only partially(see my comments).

@Eliah-Lakhin
Copy link
Contributor

Windows build is failing due to https://status.chocolatey.org/ outage. I think this is temporary.

@Eliah-Lakhin
Copy link
Contributor

@Rua Good catch! Thanks!

@Rua Rua deleted the edition2018 branch April 27, 2021 12:13
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