-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Remove dataframes crate and feature #12889
Conversation
Great work on this PR. My main concern is that it will break winget, not due to the rust changes but due to the ci and script changes. That's what all my comments are about. I started making comments but instead of going through the changes, unrelated to rust code and specific to winget, we need to have the rust flags in order for winget to work. I couldn't remember which flags need to be maintained so I went to lookup the issues which have them and describe why they're needed. Please restore these if/where they're missing. |
Oh ok, I see the issue with the |
Hopefully winget will pass with these changes. Otherwise, I'll be pinging you to reverse the ci changes. |
I think you're one the right track with removing full, I was just worried about the rust flags parts. |
With good reason XD The I also kept the Otherwise, feel free to double check that the release workflows have the same |
It's pretty clear that all the workflows are different. Only time will tell as to whether winget-pkgs will accept our next release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I can see by looking at the workflows etc. looks all plausible to me
@@ -4,7 +4,7 @@ description = "Nushell dataframe plugin commands based on polars." | |||
edition = "2021" | |||
license = "MIT" | |||
name = "nu_plugin_polars" | |||
repository = "https://github.com/nushell/nushell/tree/main/crates/nu-cmd-dataframe" | |||
repository = "https://github.com/nushell/nushell/tree/main/crates/nu_plugin_polars" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
if we're going to land this, we should do it soon enough to be able to use it a bit before the release. |
Description
Removes the old
nu-cmd-dataframe
crate in favor of the polars plugin. As such, this PR also removes thedataframe
feature, related CI, and full releases of nushell.