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

Document cfg(version) #981

Closed
wants to merge 1 commit into from
Closed

Document cfg(version) #981

wants to merge 1 commit into from

Conversation

est31
Copy link
Member

@est31 est31 commented Mar 15, 2021

Tracking issue: rust-lang/rust#64796

@ehuss ehuss added the S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository label Mar 15, 2021
Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

Can you pick a version number that has the feature implemented? Reading this, I'd think that I could use the feature in Rust 1.50.0.

Also, can you add a Note specifying when the minimum version that understands this notation? You can leave it as X.YY.0 for now until it's stabilized, but I assume with the greatest confidence that people will be looking for that specifically or will not know it doesn't work for e.g. Rust 1.0.0.

Also, are other forms allowed? E.g. is version("false") valid?

--

There's also some tone mismatches compared to the rest of the Reference, but it'd be easier to send a PR to your repo for those than to try to explain them.

--

Otherwise, looks good and thanks for writing this ahead of time!

@est31
Copy link
Member Author

est31 commented Mar 23, 2021

Also, are other forms allowed? E.g. is version("false") valid?

It does not follow the format so the cfg will evaluate to false (and a warning will be emitted, does the reference cover warnings?). It's "allowed" in the sense that it doesn't yield an error, to be forward compatible with future changes to rust's versioning without sacrificing compatibility with older compilers. version(false) on the other hand is not allowed, it does yield errors.

@est31
Copy link
Member Author

est31 commented Mar 23, 2021

I've written the section to specifically cover the question of version("false")'s validity, but, while containing the info, apparently it didn't convey it, so improvements are needed. Suggestions welcome!

@Havvy
Copy link
Contributor

Havvy commented Mar 24, 2021

Perhaps explicitly stating "Strings that don't match this version format are allowed, and are always false."?

@est31
Copy link
Member Author

est31 commented Oct 20, 2021

FCP is cancelled so I'm closing this for the time being, but will maybe reopen or refile in the future if there is more effort towards stabilization, also addressing reviewer inputs. I also invite others to build on my work if they want to do it instead of me.

@est31 est31 closed this Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-stabilization Waiting for a stabilization PR to be merged in the main Rust repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants