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

add no-op enable_ansi_support method for other platforms #55

Closed
wants to merge 3 commits into from

Conversation

mainrs
Copy link

@mainrs mainrs commented Aug 23, 2019

This commit also moves the code base to the 2018 edition of Rust. This involved changing the import scheme of used modules. Crates do not need to be declared within lib.rs explicitly. The rather get pulled in as soon as they are imported via 'use' the first time.

This commit also moves the code base to the 2018 edition of Rust. This involved changing the import scheme of used modules. Crates do not need to be declared within lib.rs explicitly. The rather get pulled in as soon as they are imported via 'use' the first time
@mainrs
Copy link
Author

mainrs commented Aug 24, 2019

Is a minimum Rust version of 1.17 still needed and wanted? If yes, I have to revert the changes that ported the code to Rust 2018 edition. Not a huge problem 😄

@mainrs
Copy link
Author

mainrs commented Sep 2, 2019

@ogham Appveyor will probably fail as the minimum Rust version is set to 1.28.0. Is this still required or can we move to 1.31.0, the first 2018 edition release? Feedback would be appreciated as I could then either fix the appveyor configuration file or revert my changes from moving to the 2018 edition.

@joshtriplett
Copy link
Collaborator

Please don't make unrelated changes (such as upgrading to Rust 2018) in the same PR (and in this case the same commit) as a new feature. I don't see any obvious reason why this feature depends on moving to 2018.

@joshtriplett
Copy link
Collaborator

Also, can you please doublecheck that all of the path changes need to happen? At least some of them should have worked unambiguously without needing further qualification.

@joshtriplett
Copy link
Collaborator

The no-op method proposed here seems like a good idea, to avoid conditionals in the calling code, and to allow for future compatibility with other platforms that may require initialization code.

@mainrs
Copy link
Author

mainrs commented Jul 12, 2020

I will close this PR and open a new one just with the changes containing the noop.

@mainrs mainrs closed this Jul 12, 2020
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.

2 participants