Skip to content

[unstable option] required_version #3386

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

Open
scampi opened this issue Feb 13, 2019 · 11 comments
Open

[unstable option] required_version #3386

scampi opened this issue Feb 13, 2019 · 11 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for required_version

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@reynoldsbd
Copy link

Would it make sense for required_version to support SemVer semantics similar to Cargo dependencies? This would make it easier to use in cases where CI agents are updated automatically.

For instance, if somebody sets required_version = "1.4.36" but their CI agent is updated separately with a newer patch-release such as 1.4.37, currently their CI build would fail.

@tseli0s
Copy link

tseli0s commented Feb 27, 2023

Why is this not stabilized yet?

@calebcartwright
Copy link
Member

Why is this not stabilized yet?

#5365
#5367

@boozook
Copy link

boozook commented Oct 27, 2023

There should be semver match instead of just eq, I suppose.

@Awayume
Copy link

Awayume commented Nov 1, 2023

There should be semver match instead of just eq, I suppose.

I think so too.

@wesleymatosdev
Copy link
Contributor

There should be semver match instead of just eq, I suppose.

Hey guys, willing to help. Should I open a PR to it?

@ytmimi
Copy link
Contributor

ytmimi commented Feb 8, 2024

@ologbonowiwi thanks for offering to help. Before submitting a PR could you open an issue describing what you're planning to implement. I think it might be useful to discuss the approach before jumping in and working on this.

@wesleymatosdev
Copy link
Contributor

wesleymatosdev commented Feb 8, 2024

#6063 opened @ytmimi. Can you check if it makes sense?

@wesleymatosdev
Copy link
Contributor

And regarding the conditions:

  • Is the default value correct ?
  • The design and implementation of the option are sound and clean.
  • The option is well tested, both in unit tests and, optimally, in real usage.
  • There is no open bug about the option that prevents its use. (I've checked this one, no bugs open)

Which of them still needs to be worked on?

Sorry, something went wrong.

@wesleymatosdev
Copy link
Contributor

There should be semver match instead of just eq, I suppose.

I think so too.

Now we have semver match

if !check_semver_version(&required_version, version) {

Can someone help me out on drafting next steps to make this stable?

required_version now is well tested, I added a bunch of tests to make sure it matches semver spec. You can check the whole discussion/implementation on #6066.
TL;DR the only missing thing from semver spec is pre-release versions. https://github.com/dtolnay/semver does not use it, but if we decide here it's needed, I can make a contribution there to implement so we don't need to do hacky the support on this codebase.

I assume we need to review design and implementation to check if it's sound and clean. Feel free to comment on #6066 if you see anything that could be fixed, and I can create a PR.

Besides that, probably handle default value and and tests for it, afterward we should be ready to make this field stable. I'd love to hear your thoughts on it. cc: @ytmimi

@ytmimi
Copy link
Contributor

ytmimi commented Mar 5, 2025

I'd say we're still a ways away from stabilizing this option. We'll want to get feedback on real world usage. The semver change has only recently been merged, but it's not been released to nightly yet. That'll happen the next time we sync rustfmt with rust-lang/rust.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unstable option tracking issue of an unstable option
Projects
None yet
Development

No branches or pull requests

8 participants