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

Match toolkit clippy settings to CI clippy settings #10984

Merged
merged 1 commit into from Nov 9, 2023

Conversation

drbrain
Copy link
Contributor

@drbrain drbrain commented Nov 7, 2023

Description

I've had a few PRs fail clippy in CI after they pass toolkit check pr because the clippy settings are different. This brings toolkit.nu into alignment with CI and leaves notes to prompt future synchronization.

User-Facing Changes

N/A

Tests + Formatting

cargo output elided:

❯ toolkit check pr
running `toolkit fmt`
running `toolkit clippy`
running `toolkit clippy` on tests
running `toolkit clippy` on plugins
running `toolkit test`
running `toolkit test stdlib`
- :green_circle: `toolkit fmt`
- :green_circle: `toolkit clippy`
- :green_circle: `toolkit test`
- :green_circle: `toolkit test stdlib`

After Submitting

N/A

@amtoine
Copy link
Member

amtoine commented Nov 7, 2023

what are all the non-toolkit changes? 🤔

@drbrain
Copy link
Contributor Author

drbrain commented Nov 7, 2023

Ugh, I thought I had undone these when I noticed CI allows unwrap()

I've had a few PRs fail CI but pass `toolkit check pr` because there was
a settings mismatch.
@amtoine
Copy link
Member

amtoine commented Nov 7, 2023

Ugh, I thought I had undone these when I noticed CI allows unwrap()

no worries 😉

if you can revert these changes so that we can see a bit better what has changed 😇

@drbrain
Copy link
Contributor Author

drbrain commented Nov 7, 2023

Fixed it in 5e65d16

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

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

i did not even notice we had 3 clippy calls in the CI 😮

thanks @drbrain, let's try this and see if there are issues again between the toolkit and the CI 🙏

@amtoine amtoine merged commit 9192037 into nushell:main Nov 9, 2023
19 checks passed
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
# Description

I've had a few PRs fail clippy in CI after they pass `toolkit check pr`
because the clippy settings are different. This brings `toolkit.nu` into
alignment with CI and leaves notes to prompt future synchronization.

# User-Facing Changes

N/A

# Tests + Formatting

`cargo` output elided:

```
❯ toolkit check pr
running `toolkit fmt`
running `toolkit clippy`
running `toolkit clippy` on tests
running `toolkit clippy` on plugins
running `toolkit test`
running `toolkit test stdlib`
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
```

# After Submitting

N/A
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
# Description

I've had a few PRs fail clippy in CI after they pass `toolkit check pr`
because the clippy settings are different. This brings `toolkit.nu` into
alignment with CI and leaves notes to prompt future synchronization.

# User-Facing Changes

N/A

# Tests + Formatting

`cargo` output elided:

```
❯ toolkit check pr
running `toolkit fmt`
running `toolkit clippy`
running `toolkit clippy` on tests
running `toolkit clippy` on plugins
running `toolkit test`
running `toolkit test stdlib`
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
```

# After Submitting

N/A
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

2 participants