-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: deprecate ssh_skip_request_pty
#222
Conversation
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. |
There was a problem hiding this comment.
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.
Yep! I have this and the others all under the v1.1.0 milestone - we'll need to bump |
ssh_skip_request_pty
ssh_skip_request_pty
f338284
to
3bdecaa
Compare
I've updated this to simply add a deprecation warning and that it will be removed in the next major. cc @nywilken @lbajolet-hashicorp |
3bdecaa
to
0ad2d6e
Compare
There was a problem hiding this 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.
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>
0ad2d6e
to
54ccaa3
Compare
Description
Deprecate
ssh_skip_request_pty
per comment:packer-plugin-vmware/builder/vmware/common/ssh_config.go
Lines 13 to 19 in 330dc55
Testing