Skip to content

chore: deprecate ssh_skip_request_pty#222

Merged
lbajolet-hashicorp merged 1 commit intomainfrom
chore/remove-ssh_skip_request_pty
Jul 29, 2024
Merged

chore: deprecate ssh_skip_request_pty#222
lbajolet-hashicorp merged 1 commit intomainfrom
chore/remove-ssh_skip_request_pty

Conversation

@tenthirtyam
Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam commented Jul 8, 2024 •
edited
Loading

Description

Deprecate ssh_skip_request_pty per comment:

https://github.com/hashicorp/packer-plugin-vmware/blob/330dc55d3b00bab6d95ae17ae541ce55f5efb50e/builder/vmware/common/ssh_config.go#L13-L19

Testing

packer-plugin-vmware on  chore/remove-ssh_skip_request_pty [$!] via 🐹 v1.22.5 
⇣7% ➜ go fmt ./... 

packer-plugin-vmware on  chore/remove-ssh_skip_request_pty [$!] via 🐹 v1.22.5 
⇣7% ➜ make generate
2024/07/07 22:12:12 Copying "docs" to ".docs/"
2024/07/07 22:12:12 Replacing @include '...' calls in .docs/
Compiling MDX docs in '.docs' to Markdown in '.web-docs'...

packer-plugin-vmware on  chore/remove-ssh_skip_request_pty [$!] via 🐹 v1.22.5 
⇣7% ➜ make build   

packer-plugin-vmware on  chore/remove-ssh_skip_request_pty [$!] via 🐹 v1.22.5 took 3.2s 
⇣7% ➜ make test
?       github.com/hashicorp/packer-plugin-vmware       [no test files]
?       github.com/hashicorp/packer-plugin-vmware/version       [no test files]
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/common 6.603s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/iso    2.159s
ok      github.com/hashicorp/packer-plugin-vmware/builder/vmware/vmx    1.716s

@tenthirtyam tenthirtyam added this to the v1.0.12 milestone Jul 8, 2024
@tenthirtyam tenthirtyam self-assigned this Jul 8, 2024
@tenthirtyam tenthirtyam requested a review from a team as a code owner July 8, 2024 02:15
@nywilken
Copy link
Copy Markdown
Contributor

nywilken commented Jul 17, 2024 •
edited
Loading

So this change requires a minor bump. When we remove configuration options that have been deprecated a minor is used to finalize the remove. Please let me know if you have any objections.

@nywilken nywilken added the version/bump minor Version: Bump Minor label Jul 17, 2024
nywilken
nywilken approved these changes Jul 17, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@nywilken nywilken left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good to me. I am approving but will hold on the merge until I get the okay since this needs to be released in a minor version.

@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Jul 17, 2024

Yep! I have this and the others all under the v1.1.0 milestone - we'll need to bump version.go.

@tenthirtyam tenthirtyam changed the title chore: remove ssh_skip_request_pty chore: deprecate ssh_skip_request_pty Jul 24, 2024
@tenthirtyam tenthirtyam force-pushed the chore/remove-ssh_skip_request_pty branch from f338284 to 3bdecaa Compare July 24, 2024 15:34
@tenthirtyam
Copy link
Copy Markdown
Collaborator Author

tenthirtyam commented Jul 24, 2024

I've updated this to simply add a deprecation warning and that it will be removed in the next major. cc @nywilken @lbajolet-hashicorp

@tenthirtyam tenthirtyam requested a review from nywilken July 24, 2024 15:35
@tenthirtyam tenthirtyam force-pushed the chore/remove-ssh_skip_request_pty branch from 3bdecaa to 0ad2d6e Compare July 24, 2024 15:36
@tenthirtyam tenthirtyam added version/bump minor Version: Bump Minor and removed version/bump minor Version: Bump Minor labels Jul 24, 2024
lbajolet-hashicorp
lbajolet-hashicorp approved these changes Jul 26, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@lbajolet-hashicorp lbajolet-hashicorp left a comment

Choose a reason for hiding this comment

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

Overall LGTM! I left a comment regarding the removal of c.Comm.SSHPty = false, just out of an abundance of precaution here, feel free to respond when you have time.

Pre-approving to not delay a future merge, since it's a minor concern, and whatever resolution will be good imo.

@tenthirtyam
chore: deprecated ssh_skip_request_pty
54ccaa3 
Deprecated `ssh_skip_request_pty` per comment:

```
// These are deprecated, but we keep them around for BC
// TODO(@mitchellh): remove"
```

Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
@tenthirtyam tenthirtyam force-pushed the chore/remove-ssh_skip_request_pty branch from 0ad2d6e to 54ccaa3 Compare July 26, 2024 20:57
@lbajolet-hashicorp lbajolet-hashicorp merged commit 35732ba into main Jul 29, 2024
@lbajolet-hashicorp lbajolet-hashicorp deleted the chore/remove-ssh_skip_request_pty branch July 29, 2024 13:08
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 1, 2026

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nywilken nywilken Awaiting requested review from nywilken

1 more reviewer

@lbajolet-hashicorp lbajolet-hashicorp lbajolet-hashicorp approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tenthirtyam tenthirtyam

Labels

chore Chore version/bump minor Version: Bump Minor

Projects

None yet

Milestone

v1.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@tenthirtyam @nywilken @lbajolet-hashicorp