Skip to content
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

Do not include SSHKit::DSL in the context it is required in #219

Merged
merged 2 commits into from
Feb 22, 2016

Conversation

beatrichartz
Copy link
Contributor

In #28 the dsl file was removed from the original requires - I assume since the DSL was also automatically included in the context it was required in.

If I want to load the DSL and use it in another context than required in, the current inclusion pattern for the DSL leaves no options - the DSL gets auto-included wherever I require it.

This leads to code like:

require 'sskit'
module MyModule
  class ClassUsingDSL
    require 'sshkit/dsl'
    #...
  end
end

which is suboptimal. We can require the dsl with all other files and make the include optional instead, which allows for:

require 'sshkit'
module MyModule
  class ClassUsingDSL
    include SSHKit::DSL
    #...
  end
end

The required changes are in this PR.

@beatrichartz beatrichartz force-pushed the no_include_on_require branch 3 times, most recently from 6d34f67 to 31d767a Compare January 26, 2015 17:50
@leehambley
Copy link
Member

@mattbrictson any reasons you see not to merge this? I missed it, but it seems like a win.

@mattbrictson
Copy link
Member

@leehambley This looks good to me. I was worried about this breaking Capistrano, but it looks like Capistrano never actually uses the SSHKit::DSL; instead it defines its own on method that does something similar. So I think that means we are safe.

We need a CHANGELOG and a rebase for this PR.

@leehambley
Copy link
Member

@beatrichartz could you take care of that please?

@beatrichartz
Copy link
Contributor Author

@leehambley done

leehambley added a commit that referenced this pull request Feb 22, 2016
Do not include SSHKit::DSL in the context it is required in
@leehambley leehambley merged commit 211a168 into capistrano:master Feb 22, 2016
@beatrichartz beatrichartz deleted the no_include_on_require branch February 23, 2016 10:29
newrelicbot pushed a commit to newrelic/newrelic-ruby-agent that referenced this pull request Mar 24, 2016
In Capistrano 1.9, SSHKit::DSL needs to be explicitly included:
capistrano/sshkit#219
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Oct 17, 2016
## [1.11.3][] (2016-09-16)

  * Fix known_hosts caching to match on the entire hostlist
    [PR #364](capistrano/sshkit#364) @byroot

## [1.11.2][] (2016-07-29)

### Bug fixes

  * Fixed a crash occurring when `Host@keys` was set to a non-Enumerable.
    @xavierholt [PR #360](capistrano/sshkit#360)

## [1.11.1][] (2016-06-17)

### Bug fixes

  * Fixed a regression in 1.11.0 that would cause
    `ArgumentError: invalid option(s): known_hosts` in some older versions of
    net-ssh. @byroot [#357](capistrano/sshkit#357)

## [1.11.0][] (2016-06-14)

### Bug fixes

  * Fixed colorized output alignment in Logger::Pretty. @xavierholt
    [PR #349](capistrano/sshkit#349)
  * Fixed a bug that prevented nested `with` calls
    [#43](capistrano/sshkit#43)

### Other changes

  * Known hosts lookup optimization is now enabled by default. @byroot

## 1.10.0 (2016-04-22)

  * You can now opt-in to caching of SSH's known_hosts file for a speed boost
    when deploying to a large fleet of servers. Refer to the
    [README](https://github.com/capistrano/sshkit/tree/v1.10.0#known-hosts-caching) for
    details. We plan to turn this on by default in a future version of SSHKit.
    [PR #330](capistrano/sshkit#330) @byroot
  * SSHKit now explicitly closes its pooled SSH connections when Ruby exits;
    this fixes `zlib(finalizer): the stream was freed prematurely` warnings
    [PR #343](capistrano/sshkit#343) @mattbrictson
  * Allow command map entries (`SSHKit::CommandMap#[]`) to be Procs
    [PR #310](capistrano/sshkit#310)
    @mikz

## 1.9.0

**Refer to the 1.9.0.rc1 release notes for a full list of new features, fixes,
and potentially breaking changes since SSHKit 1.8.1.** There are no changes
since 1.9.0.rc1.

## 1.9.0.rc1

### Potentially breaking changes

  * The SSHKit DSL is no longer automatically included when you `require` it.
    **This means you  must now explicitly `include SSHKit::DSL`.**
    See [PR #219](capistrano/sshkit#219) for details.
    @beatrichartz
  * `SSHKit::Backend::Printer#test` now always returns true
    [PR #312](capistrano/sshkit#312) @mikz

### New features

  * `SSHKit::Formatter::Abstract` now accepts an optional Hash of options
    [PR #308](capistrano/sshkit#308) @mattbrictson
  * Add `SSHKit::Backend.current` so that Capistrano plugin authors can refactor
    helper methods and still have easy access to the currently-executing Backend
    without having to use global variables.
  * Add `SSHKit.config.default_runner` options that allows to override default command runner.
    This option also accepts a name of the custom runner class.
  * The ConnectionPool has been rewritten in this release to be more efficient
    and have a cleaner internal API. You can still completely disable the pool
    by setting `SSHKit::Backend::Netssh.pool.idle_timeout = 0`.
    @mattbrictson @byroot [PR #328](capistrano/sshkit#328)

### Bug fixes

  * make sure working directory for commands is properly cleared after `within` blocks
    [PR #307](capistrano/sshkit#307)
    @steved
  * display more accurate string for commands with spaces being output in `Formatter::Pretty`
    [PR #304](capistrano/sshkit#304)
    @steved
    [PR #319](capistrano/sshkit#319) @mattbrictson
  * Fix a race condition experienced in JRuby that could cause multi-server
    deploys to fail. [PR #322](capistrano/sshkit#322)
    @mattbrictson
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.

3 participants