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

chore: fix clippy error and add generic CI check #8040

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions .github/workflows/test.yml
Expand Up @@ -455,6 +455,31 @@ jobs:
with:
command: check licenses

rust_clippy:
needs: [determine_jobs]
if: needs.determine_jobs.outputs.rust == 'true' ||
needs.determine_jobs.outputs.cargo_on_main == 'true' ||
needs.determine_jobs.outputs.cargo_only == 'true'
name: Rust clippy
runs-on:
- "self-hosted"
- "linux"
- "x64"
- "metal"
steps:
- name: Checkout
uses: actions/checkout@v3

- name: Setup Turborepo Environment
uses: ./.github/actions/setup-turborepo-environment
with:
github-token: "${{ secrets.GITHUB_TOKEN }}"

- name: Run cargo clippy
run: |
cargo clippy --workspace --all-targets --features rustls-tls \
-- --deny clippy::all --deny warnings --allow clippy::too_many_arguments
Copy link
Contributor

@arlyon arlyon May 21, 2024

Choose a reason for hiding this comment

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

Should we perhaps set that in the libraries themselves rather than in CI? (specifically too_many_arguments). Otherwise CI and local may disagree


turborepo_rust_check:
needs: [determine_jobs]
# We test dependency changes only on main
Expand Down
9 changes: 3 additions & 6 deletions crates/turbo-tasks-memory/src/aggregation/tests.rs
Expand Up @@ -39,10 +39,7 @@ fn find_root(mut node: NodeRef) -> NodeRef {
}
}

fn check_invariants<'a>(
ctx: &NodeAggregationContext<'a>,
node_ids: impl IntoIterator<Item = NodeRef>,
) {
fn check_invariants(ctx: &NodeAggregationContext, node_ids: impl IntoIterator<Item = NodeRef>) {
let mut queue = node_ids.into_iter().collect::<Vec<_>>();
// print(ctx, &queue[0], true);
let mut visited = HashSet::new();
Expand Down Expand Up @@ -100,7 +97,7 @@ fn check_invariants<'a>(
}

// All followers should also be connected to all uppers
let missing_uppers = uppers.iter().filter(|&upper_id| {
let mut missing_uppers = uppers.iter().filter(|&upper_id| {
if follower_uppers
.iter()
.any(|follower_upper_id| follower_upper_id == upper_id)
Expand All @@ -116,7 +113,7 @@ fn check_invariants<'a>(
false
}
});
for missing_upper in missing_uppers {
if let Some(missing_upper) = missing_uppers.next() {
let upper_value = {
let upper = ctx.node(missing_upper);
upper.guard.value
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-filewatch/src/hash_watcher.rs
Expand Up @@ -801,7 +801,7 @@ mod tests {
.unwrap();
repo.branch("test-branch", &current_commit, false).unwrap();
repo.set_head("refs/heads/test-branch").unwrap();
commit_all(&repo);
commit_all(repo);
}

#[tokio::test]
Expand Down