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

rustfmt: use nightly in CI #5742

Merged
merged 2 commits into from
Nov 8, 2023
Merged

Conversation

buffalojoec
Copy link
Contributor

This PR opts for using the nightly channel in CI for rustfmt.

Copy link
Contributor Author

buffalojoec commented Nov 7, 2023

This was referenced Nov 7, 2023
@buffalojoec buffalojoec force-pushed the 11-07-rustfmt_use_entrypoint_full_path branch from d5ada93 to f3941d6 Compare November 7, 2023 11:14
@buffalojoec buffalojoec force-pushed the 11-07-rustfmt_use_nightly_in_CI branch 2 times, most recently from 54ee72c to 7f69f0d Compare November 7, 2023 11:31
@buffalojoec buffalojoec requested a review from joncinque November 7, 2023 11:34
@buffalojoec buffalojoec marked this pull request as ready for review November 7, 2023 11:34
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Along with this change, can you add a cargo-nightly script similar to the cargo script in the monorepo? It literally just needs to do:

#!/usr/bin/env bash

here=$(dirname "$0")
source "${here}"/ci/rust-version.sh nightly
# shellcheck disable=SC2054 # rust_nightly is sourced from rust-version.sh
toolchain="$rust_nightly"
set -x
exec cargo "+${toolchain}" "${@}"

This will make it easier to run fmt or clippy since you can just call ./cargo-nightly fmt or whatever. I've long used shell history to know the right nightly version to use, but a helper script will be good for other devs too. What do you think?

joncinque
joncinque previously approved these changes Nov 7, 2023
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great!

@buffalojoec buffalojoec force-pushed the 11-07-rustfmt_use_entrypoint_full_path branch from f3941d6 to 173e279 Compare November 8, 2023 10:32
@buffalojoec buffalojoec force-pushed the 11-07-rustfmt_use_nightly_in_CI branch from f18fead to 3a1ae53 Compare November 8, 2023 10:32
Copy link
Contributor Author

buffalojoec commented Nov 8, 2023

Merge activity

Base automatically changed from 11-07-rustfmt_use_entrypoint_full_path to master November 8, 2023 20:36
@buffalojoec buffalojoec force-pushed the 11-07-rustfmt_use_nightly_in_CI branch from 3a1ae53 to 266bc51 Compare November 8, 2023 20:36
@mergify mergify bot dismissed joncinque’s stale review November 8, 2023 20:36

Pull request has been modified.

@buffalojoec buffalojoec merged commit 4025c87 into master Nov 8, 2023
5 checks passed
@buffalojoec buffalojoec deleted the 11-07-rustfmt_use_nightly_in_CI branch November 8, 2023 20:37
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