Implement minor changes from @dbalexandre's review.

- Mainly whitespace changes.

- Require the migration adding the `scope` column to the
  `personal_access_tokens` table to have downtime, since API calls will
  fail if the new code is in place, but the migration hasn't run.

- Minor refactoring - load `@scopes` in a `before_action`, since we're
  doing it in three different places.

2 files changed, 6 insertions, 45 deletions

diff --git a/db/migrate/20160823083941_add_column_scopes_to_personal_access_tokens.rb b/db/migrate/20160823083941_add_column_scopes_to_personal_access_tokens.rb
index ab7f0365603..91479de840b 100644
--- a/db/migrate/20160823083941_add_column_scopes_to_personal_access_tokens.rb
+++ b/db/migrate/20160823083941_add_column_scopes_to_personal_access_tokens.rb
@@ -1,32 +1,15 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
+# The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
+# It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
+# `[]`.
 
 class AddColumnScopesToPersonalAccessTokens < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index" or "add_column_with_default"
-  # you must disable the use of transactions as these methods can not run in an
-  # existing transaction. When using "add_concurrent_index" make sure that this
-  # method is the _only_ method called in the migration, any other changes
-  # should go in a separate migration. This ensures that upon failure _only_ the
-  # index creation fails and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
   disable_ddl_transaction!
 
   def up
-    # The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
-    # It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
-    # `[]`.
     add_column_with_default :personal_access_tokens, :scopes, :string, default: ['api'].to_yaml
   end
 
diff --git a/db/migrate/20160824121037_change_personal_access_tokens_default_back_to_empty_array.rb b/db/migrate/20160824121037_change_personal_access_tokens_default_back_to_empty_array.rb
index 018cc3d4747..c8ceb116b8a 100644
--- a/db/migrate/20160824121037_change_personal_access_tokens_default_back_to_empty_array.rb
+++ b/db/migrate/20160824121037_change_personal_access_tokens_default_back_to_empty_array.rb
@@ -1,39 +1,17 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
+# The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
+# It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
+# `[]`.
 
 class ChangePersonalAccessTokensDefaultBackToEmptyArray < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
-  # When a migration requires downtime you **must** uncomment the following
-  # constant and define a short and easy to understand explanation as to why the
-  # migration requires downtime.
-  # DOWNTIME_REASON = ''
-
-  # When using the methods "add_concurrent_index" or "add_column_with_default"
-  # you must disable the use of transactions as these methods can not run in an
-  # existing transaction. When using "add_concurrent_index" make sure that this
-  # method is the _only_ method called in the migration, any other changes
-  # should go in a separate migration. This ensures that upon failure _only_ the
-  # index creation fails and can be retried or reverted easily.
-  #
-  # To disable transactions uncomment the following line and remove these
-  # comments:
-  # disable_ddl_transaction!
-
   def up
-    # The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
-    # It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
-    # `[]`.
     change_column_default :personal_access_tokens, :scopes, [].to_yaml
   end
 
   def down
-    # The default needs to be `[]`, but all existing access tokens need to have `scopes` set to `['api']`.
-    # It's easier to achieve this by adding the column with the `['api']` default, and then changing the default to
-    # `[]`.
     change_column_default :personal_access_tokens, :scopes, ['api'].to_yaml
   end
 end