Skip to content

chore: address PR #905 review feedback#908

Open
rysweet wants to merge 1 commit intomainfrom
chore/review-feedback-905
Open

chore: address PR #905 review feedback#908
rysweet wants to merge 1 commit intomainfrom
chore/review-feedback-905

Conversation

@rysweet
Copy link
Copy Markdown
Owner

@rysweet rysweet commented Mar 27, 2026

Summary

Addresses non-blocking suggestions from PR #905 reviews (code, security, philosophy).

Changes

# Feedback Action
1 Two unwrap() calls should use expect() (philosophy review) Replaced with expect("remote path must contain ':'") in cmd_sync_ops.rs
2 Trailing blank line after dead code removal (code review) Removed extra blank line in sync_helpers.rs
3 --type all shows last N per file (code review) No action — correct tail semantics, documented
4 --follow + --type all interleaving (code review) No action — documented with warning

Testing

  • All 66 parity tests pass
  • All sync_helpers tests pass
  • Pre-existing unrelated test failure in test_dispatch_session_set_get_clear (not affected by this change)

@github-actions
Copy link
Copy Markdown
Contributor

Dependency Review

Note: PR #908 is not a dependency update PR. It is a code quality and review feedback PR authored by the repository owner. No dependency changes were detected.

PR Summary

Title: chore: address PR #905 review feedback
Type: Code quality / maintenance
Files changed: 3 (pyproject.toml, cmd_sync_ops.rs, sync_helpers.rs)
Additions: 3 | Deletions: 5

Changes Analysis

File Change Type
pyproject.toml Version bump 2.7.02.7.1 Patch version increment
rust/crates/azlin/src/cmd_sync_ops.rs unwrap()expect("remote path must contain ':'") Code robustness improvement
rust/crates/azlin/src/sync_helpers.rs Remove trailing blank lines in test module Cosmetic cleanup

Risk Assessment

  • Breaking changes: No
  • Security impact: The unwrap()expect() change is a minor improvement — both panic on None, but expect() provides a clearer panic message. No security surface change.
  • Dependency changes: None

Recommendation

This PR is low-risk and addresses legitimate code review feedback. It is ready to merge once CI passes.

Generated by Dependency Review and Prioritization for issue #908

- Replace 2 unwrap() calls with expect() in cmd_sync_ops.rs for
  self-documenting panic messages (philosophy review suggestion)
- Remove trailing blank line in sync_helpers.rs (cosmetic cleanup)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rysweet rysweet force-pushed the chore/review-feedback-905 branch from 5fff09d to a4d67ce Compare March 27, 2026 19:52
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.

1 participant