Skip to content

Update MSRV to 1.54.0#2645

Closed
pksunkara wants to merge 4 commits intomasterfrom
msrv
Closed

Update MSRV to 1.54.0#2645
pksunkara wants to merge 4 commits intomasterfrom
msrv

Conversation

@pksunkara
Copy link
Member

@pksunkara pksunkara commented Jul 30, 2021

No description provided.

epage and others added 4 commits July 30, 2021 10:19
@epage @pksunkara
Bump MSRV to 1.54.0
241d183 
- This makes it so `doc` compiles on stable

Fixes #2618
@epage @pksunkara
fix(docs): Ensure examples link works from rustdoc
758e5f3
@epage @pksunkara
fix(derive): Reduce compiler error noise
52d3942 
It looks like CI hasn't been running on this and we've introduced some
problems.  It looks like we had an off-by-one error in the check for
MSRV for deciding to run ui tests.
@pksunkara
Remove doc feature
81e9370
@pksunkara pksunkara linked an issue Jul 30, 2021 that may be closed by this pull request
Replace doc(include) due to feature removal #2618
Closed
2 tasks
@pksunkara pksunkara mentioned this pull request Jul 30, 2021
Closed
@pksunkara
Copy link
Member Author

pksunkara commented Jul 30, 2021 •
edited
Loading

Need to fix the doc tests in README

@epage
Copy link
Member

epage commented Jul 30, 2021

Need to fix the doc tests in README

From my PR

I tried making it so the README is always pulled in, since that can work on our MSRV, but that breaks the no-feature CI run (examples use clap_derive). So instead I added doc to the CI to make sure the README is tested.

I'm not aware of a way to make doctests conditioned on a feature flag (like derive), so I kept the feature flag around.

And if that is the only thing blocking moving this forward, any further investigation can be split out into a separate PR. It isn't blocking for bumping MSRV.

@pksunkara
Copy link
Member Author

pksunkara commented Jul 30, 2021

The main reason to bump the MSRV is to make sure the README.md is always pulled in. Otherwise, we didn't need it in the first place.

@epage
Copy link
Member

epage commented Jul 30, 2021

We can make doc a default feature.

@pksunkara
Copy link
Member Author

pksunkara commented Jul 30, 2021

What's the reason for having a doc feature? I did it because of unstable support for external_doc. Now, it's not needed anymore.

@epage
Copy link
Member

epage commented Jul 30, 2021

What's the reason for having a doc feature? I did it because of unstable support for external_doc. Now, it's not needed anymore.

When someone (CI or user) wants to run tests with --no-default-features, README doctests will fail because they depend on feature. When its a normal test, you can have a #[cfg()] but I'm not aware of a way of doing that with doctests.

By keeping doc, we can make it so CI and users have a way to control when the README doctests are run.

@pksunkara
Copy link
Member Author

pksunkara commented Jul 30, 2021

There are ways to do that but unfortunately they don't work when we do include_str!.

@pksunkara pksunkara closed this Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Replace doc(include) due to feature removal

2 participants

@pksunkara @epage