E569
Skip to content

[Refactor] Removing deadcode#201

Merged
jj-devhub merged 4 commits intomainfrom
refactor/deadcode
Sep 29, 2025
Merged

[Refactor] Removing deadcode#201
jj-devhub merged 4 commits intomainfrom
refactor/deadcode

Conversation

@tanimahossain-infiniti
Copy link
Copy Markdown
Contributor
@tanimahossain-infiniti tanimahossain-infiniti commented Sep 15, 2025
  • Removed unused self reference of classes
  • Added backticks
  • Formatting Issue
  • Removed unused functions

@tanimahossain-infiniti tanimahossain-infiniti marked this pull request as ready for review September 15, 2025 15:47
@tanimahossain-infiniti tanimahossain-infiniti added the refactor Refactored code base label Sep 15, 2025
@yeahia-sarker yeahia-sarker changed the title [Refactor] deadcode [Refactor] Removing deadcode Sep 15, 2025
Copy link
Copy Markdown
Contributor
@jj-devhub jj-devhub left a comment

Choose a reason for hiding this comment

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

Changes already made looks good but kindly look into the one review put

Comment on lines -888 to -900
// Build a set of keys to include: each parent by id and by name (if known)
let mut include_keys: Vec<String> = Vec::new();
if let Some(map) = id_name_obj {
for pid in &parent_ids {
include_keys.push(pid.clone());
if let Some(name_val) = map.get(pid).and_then(|v| v.as_str()) {
include_keys.push(name_val.to_string());
}
}
} else {
include_keys.extend(parent_ids.iter().cloned());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why removed? May impact into breaking change possibly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This part only mutates include_keys which is never used after. No reading, no mutating, no reference creating. these are the only times the include_keys appear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok will need to reintroduce this with other related implementations

@jj-devhub jj-devhub self-requested a review September 29, 2025 07:44
@jj-devhub jj-devhub merged commit 3dad4f4 into main Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Refactored code base

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0