From 97fbc2e5cd2dece8575a0d26f2db4fc502e5a9ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 19 Sep 2018 18:51:21 +0200 Subject: [PATCH 01/11] Add index on pipelines to project id and source --- ...9_add_index_pipelines_project_id_source.rb | 20 +++++++++++++++++++ db/schema.rb | 1 + 2 files changed, 21 insertions(+) create mode 100644 db/migrate/20180916011959_add_index_pipelines_project_id_source.rb diff --git a/db/migrate/20180916011959_add_index_pipelines_project_id_source.rb b/db/migrate/20180916011959_add_index_pipelines_project_id_source.rb new file mode 100644 index 00000000000..b9bebf30cf0 --- /dev/null +++ b/db/migrate/20180916011959_add_index_pipelines_project_id_source.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexPipelinesProjectIdSource < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :ci_pipelines, [:project_id, :source] + end + + def down + remove_concurrent_index :ci_pipelines, [:project_id, :source] + end +end diff --git a/db/schema.rb b/db/schema.rb index f92d8005dfb..ecb9d4391d7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -475,6 +475,7 @@ ActiveRecord::Schema.define(version: 20180917172041) do add_index "ci_pipelines", ["project_id", "iid"], name: "index_ci_pipelines_on_project_id_and_iid", unique: true, where: "(iid IS NOT NULL)", using: :btree add_index "ci_pipelines", ["project_id", "ref", "status", "id"], name: "index_ci_pipelines_on_project_id_and_ref_and_status_and_id", using: :btree add_index "ci_pipelines", ["project_id", "sha"], name: "index_ci_pipelines_on_project_id_and_sha", using: :btree + add_index "ci_pipelines", ["project_id", "source"], name: "index_ci_pipelines_on_project_id_and_source", using: :btree add_index "ci_pipelines", ["project_id", "status", "config_source"], name: "index_ci_pipelines_on_project_id_and_status_and_config_source", using: :btree add_index "ci_pipelines", ["project_id"], name: "index_ci_pipelines_on_project_id", using: :btree add_index "ci_pipelines", ["status"], name: "index_ci_pipelines_on_status", using: :btree From c2fbd8677c27b28d68efab3bfd097a246e083b98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 19 Sep 2018 18:54:03 +0200 Subject: [PATCH 02/11] Add background migration to fill pipeline source --- .../populate_external_pipeline_source.rb | 54 +++++++++++++++++++ .../populate_external_pipeline_source_spec.rb | 35 ++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 lib/gitlab/background_migration/populate_external_pipeline_source.rb create mode 100644 spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb diff --git a/lib/gitlab/background_migration/populate_external_pipeline_source.rb b/lib/gitlab/background_migration/populate_external_pipeline_source.rb new file mode 100644 index 00000000000..c2686d5fd27 --- /dev/null +++ b/lib/gitlab/background_migration/populate_external_pipeline_source.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +# rubocop:disable Style/Documentation + +module Gitlab + module BackgroundMigration + class PopulateExternalPipelineSource + module Migratable + class Pipeline < ActiveRecord::Base + self.table_name = 'ci_pipelines' + + def self.sources + { + unknown: nil, + push: 1, + web: 2, + trigger: 3, + schedule: 4, + api: 5, + external: 6 + } + end + end + + class CommitStatus < ActiveRecord::Base + self.table_name = 'ci_builds' + self.inheritance_column = :_type_disabled + end + + class Build < CommitStatus + end + + class GenericCommitStatus < CommitStatus + end + end + + def perform(start_id, stop_id) + external_pipelines(start_id, stop_id).each do |pipeline| + pipeline.update_attribute(:source, Migratable::Pipeline.sources[:external]) + end + end + + private + + def external_pipelines(start_id, stop_id) + Migratable::Pipeline.where(id: (start_id..stop_id)) + .where( + 'EXISTS (?) AND NOT EXISTS (?)', + Migratable::GenericCommitStatus.where("type='CommitStatus'").where('ci_builds.commit_id=ci_pipelines.id').select(1), + Migratable::Build.where("type='Ci::Build'").where('ci_builds.commit_id=ci_pipelines.id').select(1) + ) + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb new file mode 100644 index 00000000000..71cdc5e364c --- /dev/null +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do + let(:migration) { described_class.new } + let(:projects) { table(:projects) } + let(:pipelines) { table(:ci_pipelines) } + let(:statuses) { table(:ci_builds) } + let(:builds) { table(:ci_builds) } + + let!(:internal_pipeline) { pipelines.create(id: 1, source: described_class::Migratable::Pipeline.sources[:web]) } + let!(:external_pipeline) { pipelines.create(id: 2, source: nil) } + let!(:second_external_pipeline) { pipelines.create(id: 3, source: nil) } + let!(:status) { statuses.create(id: 1, commit_id: 2, type: 'CommitStatus') } + let!(:build) { builds.create(id: 2, commit_id: 1, type: 'Ci::Build') } + + subject { migration.perform(1, 2) } + + it 'populates the pipeline source' do + subject + + expect(external_pipeline.reload.source).to eq(described_class::Migratable::Pipeline.sources[:external]) + end + + it 'only processes a single batch of links at a time' do + subject + + expect(second_external_pipeline.reload.source).to eq(nil) + end + + it 'can be repeated without effect' do + subject + + expect { subject }.not_to change { external_pipeline.reload.source } + end +end From a6ffefc73f65e0662a976b713785e0134b64cbb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 19 Sep 2018 18:54:54 +0200 Subject: [PATCH 03/11] Schedule background migration --- ...14356_populate_external_pipeline_source.rb | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 db/post_migrate/20180916014356_populate_external_pipeline_source.rb diff --git a/db/post_migrate/20180916014356_populate_external_pipeline_source.rb b/db/post_migrate/20180916014356_populate_external_pipeline_source.rb new file mode 100644 index 00000000000..5577d05cf40 --- /dev/null +++ b/db/post_migrate/20180916014356_populate_external_pipeline_source.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class PopulateExternalPipelineSource < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + MIGRATION = 'PopulateExternalPipelineSource'.freeze + BATCH_SIZE = 500 + + disable_ddl_transaction! + + class Pipeline < ActiveRecord::Base + include EachBatch + self.table_name = 'ci_pipelines' + end + + def up + Pipeline.where(source: nil).tap do |relation| + queue_background_migration_jobs_by_range_at_intervals(relation, + MIGRATION, + 5.minutes, + batch_size: BATCH_SIZE) + end + end + + def down + # noop + end +end From 926be9029f910aac8de7fe531b3ff38c0d31f83f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 19 Sep 2018 18:56:15 +0200 Subject: [PATCH 04/11] Add CHANGELOG entry --- .../51651-fill-pipeline-source-for-external-pipelines.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/51651-fill-pipeline-source-for-external-pipelines.yml diff --git a/changelogs/unreleased/51651-fill-pipeline-source-for-external-pipelines.yml b/changelogs/unreleased/51651-fill-pipeline-source-for-external-pipelines.yml new file mode 100644 index 00000000000..6720430e5fc --- /dev/null +++ b/changelogs/unreleased/51651-fill-pipeline-source-for-external-pipelines.yml @@ -0,0 +1,5 @@ +--- +title: Retroactively fill pipeline source for external pipelines. +merge_request: 21814 +author: +type: other From 2122a7707c3c9f44f44534dea16812db5c2b97da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 19 Sep 2018 19:13:48 +0200 Subject: [PATCH 05/11] Add frozen string literal comment --- .../populate_external_pipeline_source_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index 71cdc5e364c..8e918daefb5 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do From 6075a9323a228dfdb4aca7a99861e51c8988cc56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 27 Sep 2018 18:57:19 +0200 Subject: [PATCH 06/11] Update all pipelines in single query --- .../populate_external_pipeline_source.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/background_migration/populate_external_pipeline_source.rb b/lib/gitlab/background_migration/populate_external_pipeline_source.rb index c2686d5fd27..f635064cac5 100644 --- a/lib/gitlab/background_migration/populate_external_pipeline_source.rb +++ b/lib/gitlab/background_migration/populate_external_pipeline_source.rb @@ -34,9 +34,8 @@ module Gitlab end def perform(start_id, stop_id) - external_pipelines(start_id, stop_id).each do |pipeline| - pipeline.update_attribute(:source, Migratable::Pipeline.sources[:external]) - end + external_pipelines(start_id, stop_id) + .update_all(:source, Migratable::Pipeline.sources[:external]) end private From 4de93b6a3e4eabd3eae9eb23910c6acd62649cfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 27 Sep 2018 19:28:24 +0200 Subject: [PATCH 07/11] Refactor external_pipelines query --- .../populate_external_pipeline_source.rb | 13 +++++-------- .../populate_external_pipeline_source_spec.rb | 2 +- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/background_migration/populate_external_pipeline_source.rb b/lib/gitlab/background_migration/populate_external_pipeline_source.rb index f635064cac5..98eb7f0b897 100644 --- a/lib/gitlab/background_migration/populate_external_pipeline_source.rb +++ b/lib/gitlab/background_migration/populate_external_pipeline_source.rb @@ -24,18 +24,15 @@ module Gitlab class CommitStatus < ActiveRecord::Base self.table_name = 'ci_builds' self.inheritance_column = :_type_disabled - end - class Build < CommitStatus - end - - class GenericCommitStatus < CommitStatus + scope :has_pipeline, -> { where('ci_builds.commit_id=ci_pipelines.id') } + scope :of_type, -> (type) { where('type=?', type) } end end def perform(start_id, stop_id) external_pipelines(start_id, stop_id) - .update_all(:source, Migratable::Pipeline.sources[:external]) + .update_all(source: Migratable::Pipeline.sources[:external]) end private @@ -44,8 +41,8 @@ module Gitlab Migratable::Pipeline.where(id: (start_id..stop_id)) .where( 'EXISTS (?) AND NOT EXISTS (?)', - Migratable::GenericCommitStatus.where("type='CommitStatus'").where('ci_builds.commit_id=ci_pipelines.id').select(1), - Migratable::Build.where("type='Ci::Build'").where('ci_builds.commit_id=ci_pipelines.id').select(1) + Migratable::CommitStatus.of_type('GenericCommitStatus').has_pipeline.select(1), + Migratable::CommitStatus.of_type('Ci::Build').has_pipeline.select(1), ) end end diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index 8e918daefb5..d3926ca4e7c 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -12,7 +12,7 @@ describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration let!(:internal_pipeline) { pipelines.create(id: 1, source: described_class::Migratable::Pipeline.sources[:web]) } let!(:external_pipeline) { pipelines.create(id: 2, source: nil) } let!(:second_external_pipeline) { pipelines.create(id: 3, source: nil) } - let!(:status) { statuses.create(id: 1, commit_id: 2, type: 'CommitStatus') } + let!(:status) { statuses.create(id: 1, commit_id: 2, type: 'GenericCommitStatus') } let!(:build) { builds.create(id: 2, commit_id: 1, type: 'Ci::Build') } subject { migration.perform(1, 2) } From 3e309ea70dd3c91a618a0b59b386244d44ec594a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 28 Sep 2018 00:41:24 +0200 Subject: [PATCH 08/11] Refactor migration specs using ActiveRecord models --- .../populate_external_pipeline_source.rb | 2 +- .../populate_external_pipeline_source_spec.rb | 29 +++++++++++-------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/gitlab/background_migration/populate_external_pipeline_source.rb b/lib/gitlab/background_migration/populate_external_pipeline_source.rb index 98eb7f0b897..036fe641757 100644 --- a/lib/gitlab/background_migration/populate_external_pipeline_source.rb +++ b/lib/gitlab/background_migration/populate_external_pipeline_source.rb @@ -42,7 +42,7 @@ module Gitlab .where( 'EXISTS (?) AND NOT EXISTS (?)', Migratable::CommitStatus.of_type('GenericCommitStatus').has_pipeline.select(1), - Migratable::CommitStatus.of_type('Ci::Build').has_pipeline.select(1), + Migratable::CommitStatus.of_type('Ci::Build').has_pipeline.select(1) ) end end diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index d3926ca4e7c..d95077beb59 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -4,29 +4,34 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do let(:migration) { described_class.new } - let(:projects) { table(:projects) } - let(:pipelines) { table(:ci_pipelines) } - let(:statuses) { table(:ci_builds) } - let(:builds) { table(:ci_builds) } - let!(:internal_pipeline) { pipelines.create(id: 1, source: described_class::Migratable::Pipeline.sources[:web]) } - let!(:external_pipeline) { pipelines.create(id: 2, source: nil) } - let!(:second_external_pipeline) { pipelines.create(id: 3, source: nil) } - let!(:status) { statuses.create(id: 1, commit_id: 2, type: 'GenericCommitStatus') } - let!(:build) { builds.create(id: 2, commit_id: 1, type: 'Ci::Build') } + let!(:internal_pipeline) { create(:ci_pipeline, source: Ci::Pipeline.sources[:web]) } + let!(:external_pipeline) do + build(:ci_pipeline, source: Ci::Pipeline.sources[:unknown]) + .tap { |pipeline| pipeline.save(validate: false) } + end + let!(:second_external_pipeline) do + build(:ci_pipeline, source: Ci::Pipeline.sources[:unknown]) + .tap { |pipeline| pipeline.save(validate: false) } + end - subject { migration.perform(1, 2) } + before do + create(:generic_commit_status, pipeline: external_pipeline) + create(:ci_build, pipeline: internal_pipeline) + end + + subject { migration.perform(external_pipeline.id, second_external_pipeline.id) } it 'populates the pipeline source' do subject - expect(external_pipeline.reload.source).to eq(described_class::Migratable::Pipeline.sources[:external]) + expect(external_pipeline.reload.source).to eq('external') end it 'only processes a single batch of links at a time' do subject - expect(second_external_pipeline.reload.source).to eq(nil) + expect(second_external_pipeline.reload.source).to eq('unknown') end it 'can be repeated without effect' do From 0b7ff7c57f2c12bdd401dafe6f9addf29e8d348f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 28 Sep 2018 23:47:55 +0200 Subject: [PATCH 09/11] Restyle background_migration spec --- .../populate_external_pipeline_source_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index d95077beb59..1a1193fa1fa 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -5,13 +5,14 @@ require 'spec_helper' describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration, schema: 20180916011959 do let(:migration) { described_class.new } - let!(:internal_pipeline) { create(:ci_pipeline, source: Ci::Pipeline.sources[:web]) } + let!(:internal_pipeline) { create(:ci_pipeline, source: :web) } + let!(:external_pipeline) do - build(:ci_pipeline, source: Ci::Pipeline.sources[:unknown]) + build(:ci_pipeline, source: :unknown) .tap { |pipeline| pipeline.save(validate: false) } end let!(:second_external_pipeline) do - build(:ci_pipeline, source: Ci::Pipeline.sources[:unknown]) + build(:ci_pipeline, source: :unknown) .tap { |pipeline| pipeline.save(validate: false) } end From c0716421fac7494bfad058f2608095687be49773 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 28 Sep 2018 23:56:56 +0200 Subject: [PATCH 10/11] Refactor pipeline id ordering --- .../populate_external_pipeline_source_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index 1a1193fa1fa..05a95cb5ba4 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -6,6 +6,7 @@ describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration let(:migration) { described_class.new } let!(:internal_pipeline) { create(:ci_pipeline, source: :web) } + let(:pipelines) { [internal_pipeline, external_pipeline, second_external_pipeline].map(&:id) } let!(:external_pipeline) do build(:ci_pipeline, source: :unknown) @@ -21,7 +22,7 @@ describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration create(:ci_build, pipeline: internal_pipeline) end - subject { migration.perform(external_pipeline.id, second_external_pipeline.id) } + subject { migration.perform(pipelines.min, pipelines.max) } it 'populates the pipeline source' do subject From eac01d4f658be7b4f5a857c54700a685ada8d1f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 29 Sep 2018 00:11:06 +0200 Subject: [PATCH 11/11] Reorganize background_migration specs This also adds specs for 3 distinct situations: 2. When there is an unknown pipeline with just a build 3. When there is an unknown pipeline with no statuses 1. When there is an unknown pipeline with a build and a status --- .../populate_external_pipeline_source_spec.rb | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb index 05a95cb5ba4..c7b272cd6ca 100644 --- a/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_external_pipeline_source_spec.rb @@ -6,39 +6,57 @@ describe Gitlab::BackgroundMigration::PopulateExternalPipelineSource, :migration let(:migration) { described_class.new } let!(:internal_pipeline) { create(:ci_pipeline, source: :web) } - let(:pipelines) { [internal_pipeline, external_pipeline, second_external_pipeline].map(&:id) } + let(:pipelines) { [internal_pipeline, unknown_pipeline].map(&:id) } - let!(:external_pipeline) do + let!(:unknown_pipeline) do build(:ci_pipeline, source: :unknown) .tap { |pipeline| pipeline.save(validate: false) } end - let!(:second_external_pipeline) do - build(:ci_pipeline, source: :unknown) - .tap { |pipeline| pipeline.save(validate: false) } - end - - before do - create(:generic_commit_status, pipeline: external_pipeline) - create(:ci_build, pipeline: internal_pipeline) - end subject { migration.perform(pipelines.min, pipelines.max) } - it 'populates the pipeline source' do - subject - - expect(external_pipeline.reload.source).to eq('external') + shared_examples 'no changes' do + it 'does not change the pipeline source' do + expect { subject }.not_to change { unknown_pipeline.reload.source } + end end - it 'only processes a single batch of links at a time' do - subject + context 'when unknown pipeline is external' do + before do + create(:generic_commit_status, pipeline: unknown_pipeline) + end - expect(second_external_pipeline.reload.source).to eq('unknown') + it 'populates the pipeline source' do + subject + + expect(unknown_pipeline.reload.source).to eq('external') + end + + it 'can be repeated without effect' do + subject + + expect { subject }.not_to change { unknown_pipeline.reload.source } + end end - it 'can be repeated without effect' do - subject + context 'when unknown pipeline has just a build' do + before do + create(:ci_build, pipeline: unknown_pipeline) + end - expect { subject }.not_to change { external_pipeline.reload.source } + it_behaves_like 'no changes' + end + + context 'when unknown pipeline has no statuses' do + it_behaves_like 'no changes' + end + + context 'when unknown pipeline has a build and a status' do + before do + create(:generic_commit_status, pipeline: unknown_pipeline) + create(:ci_build, pipeline: unknown_pipeline) + end + + it_behaves_like 'no changes' end end