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

Update CSV dependency and remove is_terminal #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

loewenheim
Copy link

The is_terminal documentation says

As of Rust 1.70, most users should use the IsTerminal trait in the Rust standard library instead of this crate.

@JarvisCraft
Copy link

JarvisCraft commented Nov 21, 2023

@phsym, could you please take a look at this PR? It seems to be minimalistic and it has a significant improvement of removing the (now-) redundant dependency.

@JarvisCraft
Copy link

@loewenheim, it seems like you should also update the lock field in the child fuzz directory.

@pinkforest
Copy link
Collaborator

This would raise MSRV too much.

@pinkforest pinkforest closed this Nov 21, 2023
@pinkforest
Copy link
Collaborator

pinkforest commented Nov 21, 2023

Ah this had two changes and I didn't mean to close this - Also I guess we can bump MSRV in a year but not now.

Can we make this only csv crate bump PR ? Did csv bump also bump MSRV ? Have to check.

@pinkforest pinkforest reopened this Nov 21, 2023
@JarvisCraft
Copy link

JarvisCraft commented Nov 21, 2023

csv has its MSRV explicitly set to 1.61.

As for the is-terminal removal (which is my personal focus), the std feature has been stabilized in 1.70.
If supporting pre-1.70 versions is a requirement for now, it may be an option to make the implementation feature-toggleable (I would prefer opt-in so that std is the default, and fallback implementation should be explicitly enabled): this way it would be possible to support older targets, while not forcing is-terminal and it's dependency tree on fresh targets. Tell me of you prefer this approach, in which case I will open a separate PR for this.

Actually, my personal view here is that such low MSRV is not actually needed here, considering that the crate seems to be primarily used with binary crates, but since it's your policy, it's just my user-based opinion on this :)

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.

None yet

3 participants