Skip to content

Conversation

lynzrand
Copy link
Collaborator

@lynzrand lynzrand commented Oct 17, 2025

  • moon bundle --all didn't work
  • moon bundle should generate metadata to feed to the IDE & tools

Atop #1096, #1097.

Copy link

semanticdiff-com bot commented Oct 17, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  crates/moon/src/cli/bundle.rs  75% smaller

@lynzrand lynzrand force-pushed the rupes-recta/bundle-logic branch from 141293b to 5979a6e Compare October 20, 2025 03:50
Copy link

Potential logic error in target handling when --all flag is used

Category
Correctness
Code Snippet
let mut surface_targets = target.clone().unwrap();
if cmd.all {
surface_targets.push(SurfaceTarget::All);
}
Recommendation
Remove the redundant push since target is already set to Some(vec![SurfaceTarget::All]) when cmd.all is true
Reasoning
When cmd.all is true, target is already set to Some(vec![SurfaceTarget::All]), so pushing SurfaceTarget::All again creates a duplicate entry in the vector, which could lead to unnecessary duplicate processing

Code duplication in target backend handling logic

Category
Maintainability
Code Snippet
if cmd.all || cmd.build_flags.target == Some(vec![SurfaceTarget::All]) {
targets.push(TargetBackend::Native);
}
Recommendation
Extract this workaround logic into a separate function with clear documentation about why this special case exists
Reasoning
The comment indicates this is a temporary workaround that should be moved elsewhere when native backend becomes stable. This logic should be centralized to make future refactoring easier

Unnecessary clone operation in target assignment

Category
Performance
Code Snippet
let mut surface_targets = target.clone().unwrap();
Recommendation
Use target.unwrap() directly or restructure to avoid the clone since target is not used after this point
Reasoning
The clone operation is unnecessary overhead since the original target variable is not used again after this assignment, and cloning vectors can be expensive for larger target lists

@lynzrand lynzrand enabled auto-merge October 20, 2025 03:50
@lynzrand lynzrand merged commit 26f36a6 into main Oct 20, 2025
11 of 12 checks passed
@lynzrand lynzrand deleted the rupes-recta/bundle-logic branch October 20, 2025 04:08
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