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

Capistrano2のrole定義で :no_release => true オプションを利用できるように変更 #9

Merged
merged 5 commits into from Mar 9, 2017

Conversation

w1mvy
Copy link
Contributor

@w1mvy w1mvy commented Mar 3, 2017

下記のようにyaml記述することで、 role :memcached, '192.168.100.1", :no_release => true という形で定義されるように変更

# memcached.yml
memcached:
  no_release: true
  hosts:
    - 192.168.100.1

@@ -1,6 +1,7 @@
class ServerSettings

class HostCollection < Array
attr_reader :role_config
Copy link
Contributor

Choose a reason for hiding this comment

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

capistranoではpropertyという呼び方をしてるので、揃えた方がわかりやすそうですね。
https://github.com/capistrano/capistrano/blob/bf45cbafe5ef396933c81f334bdbc8c6d32a38c2/spec/integration/dsl_spec.rb#L37

@@ -8,7 +8,11 @@ def self.extend(configuration)
def load_servers(pattern)
ServerSettings.load_config_dir(pattern)
ServerSettings.each_role do |role, hosts|
role role.to_sym, *hosts.map(&:host)
if hosts.class == ::ServerSettings::HostCollection && hosts.role_config["%no_release"]
Copy link
Contributor

Choose a reason for hiding this comment

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

あとはactiveやprimaryぐらいしかないみたいです。
https://github.com/capistrano/capistrano/blob/bf45cbafe5ef396933c81f334bdbc8c6d32a38c2/spec/integration/dsl_spec.rb#L13

この3パターンで分岐して書くか現在のrole_configの値を全部展開してしまっても良いかなと思いました。

@w1mvy
Copy link
Contributor Author

w1mvy commented Mar 8, 2017

@hirocaster コメント二件に関して、対応したので再度reviewお願い致します

README.md Outdated
@@ -164,7 +164,13 @@ require 'server_settings/capistrano'

load_servers("config/production/*.yaml")

```

When want to use `no_release` option, described as follow.
Copy link
Contributor

Choose a reason for hiding this comment

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

せっかく対応したので、primary, activeも記載してみてはどうでしょ?

@w1mvy
Copy link
Contributor Author

w1mvy commented Mar 8, 2017

README側も追記致しました

@hirocaster hirocaster merged commit 1356e21 into master Mar 9, 2017
@hirocaster hirocaster deleted the support-capistrano2-no-release-option branch March 9, 2017 08:11
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.

None yet

2 participants