-
Notifications
You must be signed in to change notification settings - Fork 221
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
prqlc CLI and prqlc other than CLI should be separate crates or, the cli
feature of prqlc should not be the default
#4946
Comments
We've gone back & forth on this a few times on this! I agree there are tradeoffs. It is a nicer more consistent interface to have The main tradeoffs are that consuming libraries need to have |
If so, is there any reason against publishing the CLI as |
This would be possible; e.g. |
Wouldn't the CLI functionality be degraded because CLI dependencies, such as clap, would not be kept up to date? |
Seems related to rust-lang/rfcs#3383 If the CLI can be squeezed into a feature, it means that all crates used within the CLI will be impossible to update due to restrictions imposed by non-CLI MSRVs (and version upgrades of their dependencies will be rejected because they do not justify the increase in MSRVs). |
Yes I definitely agree that's the tradeoff — if we find ourselves wanting a much newer MSRV than what we're tracking — currently we're about a year behind latest — then we should make a different crate. Otherwise it's simpler to integrate them. Is that what you're thinking? Nice find! I agree if that were implemented — particularly an auto-install per this comment — then splitting it off has fewer tradeoffs |
MSRV will become increasingly important as cargo will soon recognize the
rust-version
field and resolve dependencies.Currently the prqlc crate is a required dependency of downstream packages that want to incorporate PRQL, but the MSRV of prqlc itself is pushed up by the MSRV of the
cli
feature dependency, which is not needed downstream.MSRVs are not configurable on a per-feature basis, so as long as prqlc has the cli feature, prqlc's MSRVs are at risk of being raised by the cli feature.
This is especially troublesome for maintaining MSRV because the clap crate is not very conservative about MSRV (https://docs.rs/clap/latest/clap/#aspirations).
I think we should choose one of the following options to improve the situation here:
prqlc-cli
andprqlc
prqlc
andprqlc-core
(or something else)cli
feature from prqlc default features. And ignore the MSRV of the cli feature.The text was updated successfully, but these errors were encountered: