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

src: run clippy and fix issues #59

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

Conversation

jaudiger
Copy link

Very similar to #58 but on src folder this time

}

s
}

/// Return the unstyled length of AnsiStrings. This is equaivalent to `unstyle(strs).len()`.
/// Return the unstyled length of `AnsiStrings`. This is equivalent to `unstyle(strs).len()`.
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A typo was hidden here: equaivalent

@sholderbach
Copy link
Member

sholderbach commented May 20, 2024

Before we consider this, we are currently lacking a MSRV checking CI step.
While Nushell itself is pretty bleeding edge we have a bunch of large crates that are more conservative about MSRV as dependents (e.g. tracing-subscriber is on 1.63.0).
Out of courtesy it may be best to make sure we can maintain the MSRV until there are clear needs for more recent language features (a contentious one could be constification)

If this is not MSRV bumping I would be fine with landing it after review, but we will probably wait on shipping until a necessary bugfix/new feature arrives.

@jaudiger
Copy link
Author

I checked this morning of the two PRs I opened were raising the MSRV with the tool cargo-msrv. From my testing, the MSRV is not affected on both branches:

cargo msrv
Fetching index
Determining the Minimum Supported Rust Version (MSRV) for toolchain aarch64-apple-darwin
Using check command cargo check
Check for toolchain '1.67.1-aarch64-apple-darwin' succeeded

Check for toolchain '1.61.0-aarch64-apple-darwin' failed with:
┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ error: package `nu-ansi-term v0.50.0 (/Users/jaudiger/Development/git-repositories/jaudiger/nu-ansi-term)` cannot be built because it requires rustc 1.62.1 or newer, while the currently active rustc version is 1.61.0                                                                                                                          │
└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘
Check for toolchain '1.64.0-aarch64-apple-darwin' succeeded
Check for toolchain '1.62.1-aarch64-apple-darwin' succeeded
   Finished The MSRV is: 1.62.1

Please let me know if there is anything else I could do. If you prefer to split the PR and focus on one work at a time, I'm also okay.

@kubouch
Copy link

kubouch commented Jun 29, 2024

When I run cargo clippy in the src/ folder, nothing shows up.

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