From f83bccfe4f98281ed80c95189c5f7ed77799b2b3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 20 Jul 2016 10:59:49 +0200 Subject: [PATCH] Add minor readability, style improvements in CI config --- lib/ci/gitlab_ci_yaml_processor.rb | 5 ++--- lib/gitlab/ci/config/node/artifacts.rb | 7 ++++--- lib/gitlab/ci/config/node/attributable.rb | 2 +- lib/gitlab/ci/config/node/entry.rb | 4 ++-- lib/gitlab/ci/config/node/job.rb | 9 +++++---- lib/gitlab/ci/config/node/jobs.rb | 10 +++++----- lib/gitlab/ci/config/node/undefined.rb | 2 +- spec/lib/gitlab/ci/config/node/artifacts_spec.rb | 2 +- 8 files changed, 21 insertions(+), 20 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 013813ef00b..24601fdfe85 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -8,7 +8,8 @@ module Ci def initialize(config, path = nil) @ci_config = Gitlab::Ci::Config.new(config) - @config, @path = @ci_config.to_hash, path + @config = @ci_config.to_hash + @path = path unless @ci_config.valid? raise ValidationError, @ci_config.errors.first @@ -120,8 +121,6 @@ module Ci end def validate_job!(name, job) - raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script) - validate_job_stage!(name, job) if job[:stage] validate_job_dependencies!(name, job) if job[:dependencies] end diff --git a/lib/gitlab/ci/config/node/artifacts.rb b/lib/gitlab/ci/config/node/artifacts.rb index 2c301cf2917..844bd2fe998 100644 --- a/lib/gitlab/ci/config/node/artifacts.rb +++ b/lib/gitlab/ci/config/node/artifacts.rb @@ -9,12 +9,13 @@ module Gitlab include Validatable include Attributable - attributes :name, :untracked, :paths, :when, :expire_in + ALLOWED_KEYS = %i[name untracked paths when expire_in] + + attributes ALLOWED_KEYS validations do validates :config, type: Hash - validates :config, - allowed_keys: %i[name untracked paths when expire_in] + validates :config, allowed_keys: ALLOWED_KEYS with_options allow_nil: true do validates :name, type: String diff --git a/lib/gitlab/ci/config/node/attributable.rb b/lib/gitlab/ci/config/node/attributable.rb index 6e935c025e4..221b666f9f6 100644 --- a/lib/gitlab/ci/config/node/attributable.rb +++ b/lib/gitlab/ci/config/node/attributable.rb @@ -7,7 +7,7 @@ module Gitlab class_methods do def attributes(*attributes) - attributes.each do |attribute| + attributes.flatten.each do |attribute| define_method(attribute) do return unless config.is_a?(Hash) diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb index 813e394e51b..0c782c422b5 100644 --- a/lib/gitlab/ci/config/node/entry.rb +++ b/lib/gitlab/ci/config/node/entry.rb @@ -24,7 +24,7 @@ module Gitlab return unless valid? compose! - @entries.each_value(&:process!) + descendants.each(&:process!) end def leaf? @@ -44,7 +44,7 @@ module Gitlab end def errors - @validator.messages + @entries.values.flat_map(&:errors) + @validator.messages + descendants.flat_map(&:errors) end def value diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb index aea9fef8229..dc0813a8c18 100644 --- a/lib/gitlab/ci/config/node/job.rb +++ b/lib/gitlab/ci/config/node/job.rb @@ -9,13 +9,14 @@ module Gitlab include Configurable include Attributable + ALLOWED_KEYS = %i[tags script only except type image services allow_failure + type stage when artifacts cache dependencies before_script + after_script variables environment] + attributes :tags, :allow_failure, :when, :environment validations do - validates :config, allowed_keys: - %i[tags script only except type image services allow_failure - type stage when artifacts cache dependencies before_script - after_script variables environment] + validates :config, allowed_keys: ALLOWED_KEYS validates :config, presence: true validates :name, presence: true diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb index 908c8f4f120..51683c82ceb 100644 --- a/lib/gitlab/ci/config/node/jobs.rb +++ b/lib/gitlab/ci/config/node/jobs.rb @@ -18,10 +18,14 @@ module Gitlab end def has_visible_job? - config.any? { |key, _| !key.to_s.start_with?('.') } + config.any? { |name, _| !hidden?(name) } end end + def hidden?(name) + name.to_s.start_with?('.') + end + private def compose! @@ -37,10 +41,6 @@ module Gitlab @entries[name] = factory.create! end end - - def hidden?(name) - name.to_s.start_with?('.') - end end end end diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb index 384774c9b69..84dab61e7e9 100644 --- a/lib/gitlab/ci/config/node/undefined.rb +++ b/lib/gitlab/ci/config/node/undefined.rb @@ -5,7 +5,7 @@ module Gitlab ## # This class represents an undefined and unspecified entry node. # - # It decorates original entry adding method that idicates it is + # It decorates original entry adding method that indicates it is # unspecified. # class Undefined < SimpleDelegator diff --git a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb index 418a88cabac..beed29b18ae 100644 --- a/spec/lib/gitlab/ci/config/node/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/node/artifacts_spec.rb @@ -31,7 +31,7 @@ describe Gitlab::Ci::Config::Node::Artifacts do end end - context 'when there is uknown key' do + context 'when there is an unknown key present' do let(:config) { { test: 100 } } it 'reports error' do -- GitLab