Commit 83bbca26 authored by Douwe Maan's avatar Douwe Maan
Browse files

Merge branch 'fix-downtime-check-formatting' into 'master'

Fix downtime check formatting

## What does this MR do?

This MR adjusts the formatting of the migration downtime checker so messages are more readable.

## Are there points in the code the reviewer needs to double check?

Not specifically

## Why was this MR needed?

Formatting was somewhat hard to read and the online/offline indicators used the wrong colour.

## New Format

```
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20140407135544_fix_namespaces.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705054938_add_protected_branches_push_access.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705054952_add_protected_branches_merge_access.rb
[offline]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705055254_move_from_developers_can_merge_to_protected_branches_merge_access.rb:

We're creating a `merge_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this
is running, we might be left with a `protected_branch` _without_ an associated `merge_access_level`. The `protected_branches`
table must not change while this is running, so downtime is required.

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410

[offline]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705055308_move_from_developers_can_push_to_protected_branches_push_access.rb:

We're creating a `push_access_level` for each `protected_branch`. If a user creates a `protected_branch` while this
is running, we might be left with a `protected_branch` _without_ an associated `push_access_level`. The `protected_branches`
table must not change while this is running, so downtime is required.

https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5081#note_13247410

[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705055809_remove_developers_can_push_from_protected_branches.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160705055813_remove_developers_can_merge_from_protected_branches.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160716115711_add_queued_at_to_ci_builds.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160725083350_add_external_url_to_enviroments.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160727163552_create_user_agent_details.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160728103734_add_pipeline_events_to_services.rb
[offline]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb:

Removing a column that contains data that is not used anywhere.

[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160802010328_remove_builds_enable_index_on_projects.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160810102349_remove_ci_runner_trigram_indexes.rb
[online]: /home/yorickpeterse/Projects/gitlab/gitlab-ce/db/migrate/20160810142633_remove_redundant_indexes.rb
```

## Does this MR meet the acceptance criteria?

- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5840
parents d1da2e81 fa0624fc
module Gitlab
class DowntimeCheck
class Message
attr_reader :path, :offline, :reason
attr_reader :path, :offline
OFFLINE = "\e[32moffline\e[0m"
ONLINE = "\e[31monline\e[0m"
OFFLINE = "\e[31moffline\e[0m"
ONLINE = "\e[32monline\e[0m"
# path - The file path of the migration.
# offline - When set to `true` the migration will require downtime.
......@@ -19,10 +19,21 @@ def to_s
label = offline ? OFFLINE : ONLINE
message = "[#{label}]: #{path}"
message += ": #{reason}" if reason
if reason?
message += ":\n\n#{reason}\n\n"
end
message
end
def reason?
@reason.present?
end
def reason
@reason.strip.lines.map(&:strip).join("\n")
end
end
end
end
......@@ -5,13 +5,35 @@
it 'returns an ANSI formatted String for an offline migration' do
message = described_class.new('foo.rb', true, 'hello')
expect(message.to_s).to eq("[\e[32moffline\e[0m]: foo.rb: hello")
expect(message.to_s).to eq("[\e[31moffline\e[0m]: foo.rb:\n\nhello\n\n")
end
it 'returns an ANSI formatted String for an online migration' do
message = described_class.new('foo.rb')
expect(message.to_s).to eq("[\e[31monline\e[0m]: foo.rb")
expect(message.to_s).to eq("[\e[32monline\e[0m]: foo.rb")
end
end
describe '#reason?' do
it 'returns false when no reason is specified' do
message = described_class.new('foo.rb')
expect(message.reason?).to eq(false)
end
it 'returns true when a reason is specified' do
message = described_class.new('foo.rb', true, 'hello')
expect(message.reason?).to eq(true)
end
end
describe '#reason' do
it 'strips excessive whitespace from the returned String' do
message = described_class.new('foo.rb', true, " hello\n world\n\n foo")
expect(message.reason).to eq("hello\nworld\n\nfoo")
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment