clean up, prefer roles :all over all_roles

README.md

@@ -23,8 +23,8 @@ TODO:
   - [x] basic deploy
   - [x] ask
   - [x] add `deploy:check`
+  - [x] prefer `roles(:all)` over `all_roles`
   - [ ] simplify default deploy
-  - [ ] prefer `roles(:all)` over `all_roles`
   - [ ] support primary servers `on primary(:db)`
   - [ ] basic rollback
   - [ ] support existing significant configuration variables

lib/capistrano/configuration.rb

@@ -33,10 +33,6 @@ module Capistrano
       roles.fetch_roles(names)
     end
 
-    def all_roles
-      roles.all
-    end
-
     def configure_backend
       SSHKit.configure do |sshkit|
         sshkit.format = fetch(:format, :pretty)
@@ -116,11 +112,7 @@ module Capistrano
       end
 
       def fetch_roles(names)
-        names.map { |name| fetch name }.flatten.uniq
-      end
-
-      def all
-        roles.values.flatten.uniq
+        roles_for(names).flatten.uniq
       end
 
       def each
@@ -133,6 +125,14 @@ module Capistrano
         roles.fetch(name) { raise "role #{name} is not defined" }
       end
 
+      def roles_for(names)
+        if names.include?(:all)
+          roles.values
+        else
+          names.map { |name| fetch name }
+        end
+      end
+
       def roles
         @roles ||= Hash.new
       end

lib/capistrano/dsl.rb

@@ -12,24 +12,29 @@ module Capistrano
     include Logger
     include Stages
 
-    def invoke(task)
-      Rake::Task[task].invoke
+    def invoke(task, *args)
+      Rake::Task[task].invoke(*args)
     end
 
-    def t(*args)
-      I18n.t(*args, scope: :capistrano)
+    def t(key, options={})
+      I18n.t(key, options.merge(scope: :capistrano))
     end
 
     def scm
       fetch(:scm)
     end
 
-    def maintenance_page
-      fetch(:maintenance_page, 'public/system/maintenance.html')
+    def revision_log_message
+      t(:revision_log_message, branch: fetch(:branch), user: local_user, release: release_timestamp)
     end
 
-    def revision_log_message
-      %{Branch #{fetch(:branch)} deployed as release #{release_timestamp} by #{`whoami`}}
+    def keep_releases
+      fetch(:keep_releases, 5)
     end
+
+    def local_user
+      `whoami`
+    end
+
   end
 end

lib/capistrano/dsl/env.rb

@@ -26,10 +26,6 @@ module Capistrano
         env.roles_for(names)
       end
 
-      def all
-        env.all_roles
-      end
-
       def env
         Configuration.env
       end

lib/capistrano/i18n.rb

@@ -12,6 +12,9 @@ en = {
   stage_not_set: 'Stage not set',
   written_file: 'create %{file}',
   question: 'Please enter %{key}: |%{default_value}|',
+  keeping_releases: 'Keeping %{keep_releases} of %{releases} deployed releases',
+  linked_file_does_not_exist: 'linked file %{file} does not exist on %{host}',
+  revision_log_message: 'Branch %{branch} deployed as release %{release} by %{user}',
   console: {
     welcome: 'capistrano console - enter command to execute on %{stage}',
     bye: 'bye'

lib/capistrano/tasks/bundler.rake

@@ -1,7 +1,7 @@
 namespace :deploy do
 
   after :update, :bundle do
-    on all do
+    on roles :all do
       within release_path do
         execute :bundle, "--gemfile #{release_path}/Gemfile --deployment --binstubs #{shared_path}/bin --path #{shared_path}/bundle --without development test cucumber"
       end

lib/capistrano/tasks/console.rake

@@ -10,7 +10,7 @@ task :console do
       break
     else
       begin
-        on all do
+        on roles :all do
           execute command
         end
       rescue => e

lib/capistrano/tasks/deploy.rake

@@ -28,40 +28,30 @@ namespace :deploy do
   namespace :check do
     desc 'Check shared and release directories exist'
     task :directories do
-      on all do
-        unless test "[ -d #{shared_path} ]"
-          execute :mkdir, '-p', shared_path
-        end
-
-        unless test "[ -d #{releases_path} ]"
-          execute :mkdir, '-p', releases_path
-        end
+      on roles :all do
+        execute :mkdir, '-pv', shared_path, releases_path
       end
     end
 
     desc 'Check directories to be linked exist in shared'
     task :linked_dirs do
-      on all do
+      on roles :app do
         fetch(:linked_dirs).each do |dir|
           dir = shared_path.join(dir)
-          unless test "[ -d #{dir} ]"
-            execute :mkdir, '-p', dir
-          end
+          execute :mkdir, '-pv', dir
         end
       end
     end
 
     desc 'Check files to be linked exist in shared'
     task :linked_files do
-      on all do
+      on roles :app do |host|
         fetch(:linked_files).each do |file|
           file_path = shared_path.join(file)
           parent = file_path.dirname
-          unless test "[ -d #{parent} ]"
-            execute :mkdir, '-p', parent
-          end
+          execute :mkdir, '-pv', parent
           unless test "[ -f #{file_path} ]"
-            error "linked file #{file} does not exist"
+            error t(:linked_file_does_not_exist, file: file, host: host)
             exit 1
           end
         end
@@ -72,7 +62,7 @@ namespace :deploy do
   namespace :symlink do
     desc 'Symlink release to current'
     task :release do
-      on all do
+      on roles :app do
         execute :rm, '-rf', current_path
         execute :ln, '-s', release_path, current_path
       end
@@ -86,14 +76,12 @@ namespace :deploy do
 
     desc 'Symlink linked directories'
     task :linked_dirs do
-      on all do
+      on roles :app do
         fetch(:linked_dirs).each do |dir|
           target = release_path.join(dir)
           source = shared_path.join(dir)
           parent = target.dirname
-          unless test "[ -d #{parent} ]"
-            execute :mkdir, '-p', parent
-          end
+          execute :mkdir, '-pv', parent
           unless test "[ -L #{target} ]"
             if test "[ -f #{target} ]"
               execute :rm, '-rf', target
@@ -106,14 +94,12 @@ namespace :deploy do
 
     desc 'Symlink linked files'
     task :linked_files do
-      on all do
+      on roles :app do
         fetch(:linked_files).each do |file|
           target = release_path.join(file)
           source = shared_path.join(file)
           parent = target.dirname
-          unless test "[ -d #{parent} ]"
-            execute :mkdir, '-p', parent
-          end
+          execute :mkdir, '-pv', parent
           unless test "[ -L #{target} ]"
             if test "[ -f #{target} ]"
               execute :rm, target
@@ -127,12 +113,11 @@ namespace :deploy do
 
   desc 'Clean up old releases'
   task :cleanup do
-    on all do
-      count = fetch(:keep_releases, 5).to_i
-      releases = capture("ls -xt #{releases_path}").split.reverse
-      if releases.length >= count
-        info "keeping #{count} of #{releases.length} deployed releases"
-        directories = (releases - releases.last(count)).map { |release|
+    on roles :all do
+      release = capture(:ls, '-xt', releases_path).split.reverse
+      if releases.count >= keep_releases
+        info t(:keeping_releases, keep_releases: keep_releases, releases: releases.count)
+        directories = (releases - releases.last(keep_releases)).map { |release|
           releases_path.join(release) }.join(" ")
         execute :rm, '-rf', directories
       end
@@ -141,7 +126,7 @@ namespace :deploy do
 
   desc 'Log details of the deploy'
   task :log_revision do
-    on roles(:web) do
+    on roles :app do
       within releases_path do
         execute %{echo "#{revision_log_message}" >> #{revision_log}}
       end

lib/capistrano/tasks/git.rake

@@ -2,7 +2,7 @@ namespace :git do
 
   desc 'Check that the repository exists'
   task :check do
-    on all do
+    on roles :all do
       unless test "[ -d #{repo_path} ]"
         within deploy_path do
           execute :git, :clone, fetch(:repo), repo_path
@@ -19,7 +19,7 @@ namespace :git do
 
   desc 'Update the repo to branch or reference provided provided'
   task :reset do
-    on all do
+    on roles :all do
       within repo_path do
         execute :git, 'fetch origin'
         execute :git,  "reset --hard origin/#{fetch(:branch)}"
@@ -29,7 +29,7 @@ namespace :git do
 
   desc 'Copy repo to releases'
   task :create_release do
-    on all do
+    on roles :all do
       execute :cp, "-RPp", repo_path, release_path
     end
   end

spec/lib/capistrano/configuration_spec.rb

@@ -46,7 +46,7 @@ module Capistrano
         end
 
         it 'returns all servers' do
-          expect(roles.all).to eq [server1, server2, server3]
+          expect(roles.fetch_roles([:all])).to eq [server1, server2, server3]
         end
       end
     end

spec/lib/capistrano/dsl_spec.rb

@@ -11,7 +11,7 @@ module Capistrano
 
     describe '#t' do
       before do
-        I18n.expects(:t).with(:phrase, {count: 2}, scope: :capistrano)
+        I18n.expects(:t).with(:phrase, {count: 2, scope: :capistrano})
       end
 
       it 'delegates to I18n' do