Skip to content

Conversation

@shiavm006
Copy link
Contributor

What this PR does

Extracts duplicated TTY raw mode setup and cleanup code from Attach() and ExecStartAndAttach() into a shared setupTTYRawMode() helper function.

Changes

  • Created new setupTTYRawMode() helper function that encapsulates TTY setup logic
  • Updated Attach() to use the new helper
  • Updated ExecStartAndAttach() to use the new helper
  • Removed TODO comment that referenced this duplication

Why

This eliminates code duplication and improves maintainability. The TODO comment in ExecStartAndAttach() indicated this refactoring was needed. This follows the same pattern used elsewhere in the codebase (e.g., GetNoCompress() in pkg/machine/ocipull/ociartifact.go).

Testing

  • Code compiles successfully
  • Linting passes
  • Functionally identical to original code (refactoring only)
  • Existing tests in pkg/bindings/test/attach_test.go will run in CI

@shiavm006
Copy link
Contributor Author

hi @Luap99 This is a pure refactoring (extracting duplicated code) with no functional changes. Existing tests in pkg/bindings/test/attach_test.go and test/e2e/attach_test.go already cover the refactored code paths.

Could you please add the "No New Tests" label?

@Luap99 Luap99 added the No New Tests Allow PR to proceed without adding regression tests label Jan 7, 2026
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

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

LGTM

@baude
Copy link
Member

baude commented Jan 7, 2026

LGTM

@baude baude merged commit ed0df20 into containers:main Jan 7, 2026
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No New Tests Allow PR to proceed without adding regression tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants