Skip to content

fix typo in orion#1946

Merged
benjamin-747 merged 1 commit into
gitmono-dev:mainfrom
Ivanbeethoven:main
Feb 15, 2026
Merged

fix typo in orion#1946
benjamin-747 merged 1 commit into
gitmono-dev:mainfrom
Ivanbeethoven:main

Conversation

@Ivanbeethoven

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Luxian <lux1an@qq.com>
@github-actions

github-actions Bot commented Feb 15, 2026

Copy link
Copy Markdown

Claude finished @Ivanbeethoven's task in 1m 9s —— View job


Reviewing PR Changes

Todo List:

  • View the actual changes in the PR
  • Read the modified file for context
  • Review the typo fix for correctness
  • Check for any impact on functionality or tests
  • Provide comprehensive review feedback

Review Summary

Approved - This is a correct bug fix that addresses a parameter order mismatch.

Details

What Changed:
In orion/src/ws.rs:189-192, the parameters to buck_build() were swapped to match the correct function signature.

Before:

buck_build(
    task_id_uuid,
    repo,        // ❌ Wrong position
    cl_link,     // ❌ Wrong position
    changes,
    sender.clone(),
)

After:

buck_build(
    task_id_uuid,
    cl_link,     // ✅ Correct position
    repo,        // ✅ Correct position
    changes,
    sender.clone(),
)

Function Signature (orion/src/api.rs:45-50):

pub async fn buck_build(
    id: Uuid,
    cl_link: String,  // 2nd parameter
    repo: String,     // 3rd parameter
    changes: Vec<Status<ProjectRelativePath>>,
    sender: UnboundedSender<WSMessage>,
)

Impact Assessment

Severity: Medium - This bug would cause runtime failures when processing WebSocket build tasks.

What was broken:

  • The repo value was being passed where cl_link was expected
  • The cl_link value was being passed where repo was expected
  • This would lead to incorrect build execution paths and likely build failures

Why it compiled: Both parameters are String types, so the Rust compiler couldn't catch this semantic error at compile time.

Code Quality Review

Correctness: The fix is correct and matches the function signature
Security: No security concerns introduced
Performance: No performance impact
Testing: Consider adding integration tests to catch parameter mismatches
Documentation: The PR title accurately describes the fix

Recommendations

While this PR is good to merge, consider these improvements for the future:

  1. Type Safety: Use newtype patterns to prevent parameter confusion:

    struct ClLink(String);
    struct RepoPath(String);

    This would make parameter order mismatches a compile-time error.

  2. Testing: Add integration tests that verify the WebSocket message handling flow to catch issues like this earlier.

  3. PR Description: Future PRs would benefit from a description explaining what was wrong and how it was fixed.


@benjamin-747 benjamin-747 added this pull request to the merge queue Feb 15, 2026
Merged via the queue into gitmono-dev:main with commit 46c39f0 Feb 15, 2026
11 checks passed
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