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

Set all crates to 2021 edition #580

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Set all crates to 2021 edition #580

merged 1 commit into from
Aug 28, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Aug 20, 2023

For some reason all crates still use 2015 edition. I think it is a good idea to specify edition explicitly, or at least document why it should be kept back (and still add it to the cargo.toml for documentation purposes)

  • Remove all extern crate
  • changed use self::sval::... to use sval::..., and the same for sval_ref
  • Simplified fn get<'v>(&'v self, key: Key) -> Option<Value<'v>> to fn get(&self, key: Key) -> Option<Value<'_>>
  • Simplified many unit tests with references

Copy link
Collaborator

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

LGTM, but the CI is failing.

@nyurik
Copy link
Contributor Author

nyurik commented Aug 20, 2023

It took a few tries, but I think I fixed all the outstanding issues with this one. As the result, 50 less lines of code, and a few extra CI checks.

.github/workflows/main.yml Show resolved Hide resolved
@KodrAus
Copy link
Contributor

KodrAus commented Aug 20, 2023

Thanks for doing some much needed housekeeping @nyurik! It looks like we just have some conflicts to resolve now.

@nyurik
Copy link
Contributor Author

nyurik commented Aug 21, 2023

thx @KodrAus, rebased & fixed

@nyurik
Copy link
Contributor Author

nyurik commented Aug 28, 2023

@KodrAus hi, would you be the right person to review? I would like to make a few more improvements like use workspaces, but all other changes would be based on this one, so I don't want to make them until this is agreed on & merged. Thx!

@KodrAus KodrAus merged commit 242a982 into rust-lang:master Aug 28, 2023
14 checks passed
@KodrAus
Copy link
Contributor

KodrAus commented Aug 28, 2023

Merged! Thanks again @nyurik

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.

3 participants