Skip to content

[fix]clean code fix#1020

Merged
ygwpz merged 2 commits into
ModelEngine-Group:developfrom
qyh111:dev_clean_code
Jun 15, 2026
Merged

[fix]clean code fix#1020
ygwpz merged 2 commits into
ModelEngine-Group:developfrom
qyh111:dev_clean_code

Conversation

@qyh111

@qyh111 qyh111 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Purpose

Clean code fix

Modifications

Test

Comment thread ucm/shared/trans/cuda/gdr/gdr_copy.cc

@ygwpz ygwpz left a comment

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.

Follow-up Review

I've reviewed the new commits (4604f9c and 388a90d) since my last review on 2026-06-12.

Previous Concern Addressed ✓

Empty catch block in gdr_copy.cc: The author has added a comment explaining why the exception is ignored:

} catch (...) {
    // Ignore registration failure on existing channels, as the new registration will be
    // picked up later.
}

While I originally suggested logging the exception, this comment adequately explains the rationale for silently catching exceptions. The justification (registration will be picked up later) makes this acceptable.

New Changes Analysis

I've reviewed all the new changes across the two commits:

  1. Floating-point comparison fix (longvideo_cache.py): Changed compression_ratio == 1.0 to math.isclose(compression_ratio, 1.0, rel_tol=1e-9) - Good bug fix for proper floating-point comparison.

  2. Lambda capture improvement (io_engine_aio.h): Changed [&] to explicit captures [this, tid, w, last, id] - Good practice for clarity and safety.

  3. Single-argument constructor (trans_buffer.cc): Added explicit keyword - Best practice.

  4. Code style improvements: Added braces to single-line if statements, removed commented code, formatting improvements.

Conclusion

No blocking issues found in the new commits. The changes are primarily code quality improvements and bug fixes. The PR is ready for approval.

@qyh111 qyh111 changed the title clean code fix [fix]clean code fix Jun 15, 2026
@ygwpz ygwpz merged commit bd2967f into ModelEngine-Group:develop Jun 15, 2026
24 of 30 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