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

Modernize doc examples to use idiomatic Rust #62

Merged
merged 2 commits into from
Nov 12, 2021
Merged

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Nov 12, 2021

No description provided.

@matklad
Copy link
Contributor Author

matklad commented Nov 12, 2021

Ok, doesn't pass CI. WDYT about switching to 1.31 as MSRV (and adding edition = "2018" to the crate)?

@Gilnaa
Copy link
Owner

Gilnaa commented Nov 12, 2021

Is it possible to not run doctests in earlier targets, or does this require e2018?

@matklad
Copy link
Contributor Author

matklad commented Nov 12, 2021

That's also possible, see the last commit. I guess in a sense that is better -- you want to show-case modern Rust in docs, even if you support something very old.

Although, practically speaking, bumping to 2018 edition seems OK given that we already have 2021

@matklad
Copy link
Contributor Author

matklad commented Nov 12, 2021

image

FYI, there's a per-repository, non-default setting to make this better:

image

@Gilnaa
Copy link
Owner

Gilnaa commented Nov 12, 2021

Good idea. Though iirc you're not even a first time contributer(?).

WRT edition, you're essentially right, I can't imagine someone wanting to upgrade memoffset and not rustc, seeing as most benefits come from extremely new versions of rustc.

Will be done at some point, I think, I'll have to think about it a bit

@Gilnaa Gilnaa merged commit b14262b into Gilnaa:master Nov 12, 2021
@Gilnaa
Copy link
Owner

Gilnaa commented Nov 12, 2021

Also, thank you :)
This change was long overdue

@matklad matklad deleted the idioms branch November 12, 2021 11:16
run: cargo test
# Exclude doctests here, as we don't want to clutter docs themselves
# with backwards compatibility workarounds.
run: cargo test --lib
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some of the versions in the list above should still work here, would it be possible to still run the full test suite for them?

Also, stable/beta/nightly now are tested twice for no good reason.

@RalfJung
Copy link
Collaborator

RalfJung commented Nov 12, 2021

WDYT about switching to 1.31 as MSRV

I'd be strongly opposed to that unless all our reverse dependencies already have a higher MSRV, but it seems you managed to avoid it. :)

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