Change @_env to local variable #345
Conversation
In the SSHKit Abstract backend, the 'with' method declared an instance variable (@_env) that was only used in that method, and ensured it was unset before returning. This would cause errors when nesting 'with' calls, as the outer call would try to use and then unset @_env after the inner call already unset it. It has been changed to a regular local variable, as the desired behaviour is identical to using a local variable anyway.
|
Could you check the build please @DylanFrese ? |
|
@leehambley It looks like RuboCop is complaining about the fact that '_env' is a used variable and starts with an underscore. I did this because that's what the original variable was named (@_env), but I'll change it. |
Change the name of a local variable in the 'with' method in the Abstract Backend from '_env' to 'env_old'. Variable names beginning with underscores are typically interpreted to mean that the variable is unused. The variable name has been changed to be more descriptive.
|
@leehambley Done |
|
Thanks, LGTM I just would ask you to amend the commit again and credit yourself in the CHANGELOG if you're interested in being credited, if not just let me know and I'll merge the PR as-is. |
|
You're free to merge it as-is. |
|
@leehambley Could you please release a version with this fixed? For example capistrano whenever fails due to this bug. |
|
Yes this would be wonderful if @leehambley could fix this. My capistrano fails with the same error. Thanks. |
|
I plan on releasing 1.11.0 in a few hours, which will include this fix. |
In the SSHKit Abstract backend, the 'with' method declared an instance variable (@_env) that was only used in that method, and ensured it was unset before returning. This would cause errors when nesting 'with' calls, as the outer call would try to use and then unset @_env after the inner call already unset it. It has been changed to a regular local variable, as the desired behaviour is identical to using a local variable anyway.
This fixes Issue #43 and Issue #176