Move EE-specific files to a standalone directory

What does this MR do?

Move EE-specific files to a standalone directory

• app/{models, controllers, etc}
• lib
• app/views
• app/views/admin/monitoring/ee
• app/views/projects/protected_branches/ee
• app/views/projects/protected_tags/ee
• app/views/projects/settings/ee
• app/assets/javascripts/vue_merge_request_widget/ee
• app/assets/javascripts/protected_branches/ee
• app/assets/javascripts/protected_tags/ee
• spec
• qa

Why was this MR needed?

To make it more clear which codes are EE-specific

Does this MR meet the acceptance criteria?

• Documentation created/updated

What are the relevant issue numbers?

Closes #2902 (closed)

mentioned in issue #2902 (closed)

added 176 commits

• 6f3e033c...b6326824 - 175 commits from branch master
• c4526c1e - Move EE-specific files to a standalone directory

Compare with previous version

marked as a Work In Progress from 3119c67526ff6c0e25014533197039098a492c71

mentioned in merge request !2248 (merged)

added 153 commits

• 3119c675...833969f4 - 150 commits from branch master
• 4f90a220 - Move EE-specific files to a standalone directory
• e1efbeb7 - Also move models/concerns and lib
• ba9c6d4a - fixup! Move EE-specific files to a standalone directory

Compare with previous version

changed the description

added Edge label

assigned to @godfat

marked the checklist item app/views as completed

added 107 commits

• baa71bcb...951cd3c3 - 103 commits from branch master
• cb131925 - Move EE-specific files to a standalone directory
• e23f8690 - Also move models/concerns and lib
• 61ae3c99 - fixup! Move EE-specific files to a standalone directory
• fb4c955d - Move views

Compare with previous version

I am still not sure how we could make rspec check against ee/spec by default (see discussion over https://gitlab.com/gitlab-org/gitlab-ce/commit/adc00877adff3a3b351cf4310948ec71b6a161dc), so I think for now we could just move it to spec/ee, and after we figured out what we could do, we could just move spec/ee to ee/spec

marked the checklist item spec as completed

resolved all discussions

changed the description

added 61 commits

• d964b49c...917438cc - 57 commits from branch master
• 536dec20 - Move EE-specific files to a standalone directory
• ee122a11 - Also move models/concerns and lib
• a3994ab6 - Move views
• 5131b2d9 - Move spec files

Compare with previous version

marked the checklist item app/views/admin/monitoring/ee as completed

marked the checklist item app/views/projects/protected_branches/ee as completed

@rymai @DouweM @nick.thomas I propose we merge this soon because conflicts here might not be so easy to resolve. (fortunately never happened yet)

And I also propose that as the first step:

• Move EE tests to spec/ee/spec to avoid some difficulty related to infer_spec_type_from_file_location! (I would write in more detail if you don't know what's wrong with this)
• Leave qa/qa/ee alone because I am unsure how this works and this seems minor to me
• Leave JavaScript alone because I don't know how to deal with webpack. I am asking help at https://gitlab.com/gitlab-org/gitlab-ee/issues/2902#note_35758215
• Everything else seems to be working fine, so let's merge this first?
• There's a WIP doc which we should consider though! https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2248

What do you think?

Thanks for the awesome work here @godfat!

unmarked as a Work In Progress

marked the checklist item app/views/projects/protected_tags/ee as completed

marked the checklist item app/views/projects/settings/ee as completed

marked the checklist item app/assets/javascripts/vue_merge_request_widget/ee as completed

marked the checklist item app/assets/javascripts/protected_branches/ee as completed

marked the checklist item app/assets/javascripts/protected_tags/ee as completed

added 67 commits

• 83538775...89d7c75a - 61 commits from branch master
• e561bf3d - Move EE-specific files to a standalone directory
• 3da0787b - Also move models/concerns and lib
• 1c4215e2 - Move views
• 627b87e0 - Move spec files
• ae1bc589 - Move the new view files
• e3c67462 - Move javascript files

Compare with previous version

added 67 commits

• 83538775...89d7c75a - 61 commits from branch master
• e561bf3d - Move EE-specific files to a standalone directory
• 3da0787b - Also move models/concerns and lib
• 1c4215e2 - Move views
• 627b87e0 - Move spec files
• ae1bc589 - Move the new view files
• e3c67462 - Move javascript files

Compare with previous version

@godfat One benefit of moving EE-specific specs to ee/spec is that we could get rid of the # rubocop:disable RSpec/FilePath comments. For instance in spec/lib/gitlab/ee/visibility_level_spec.rb we describe describe Gitlab::VisibilityLevel so Rubocop is expecting the file to be spec/lib/gitlab/visibility_level_spec.rb. If we move this file to ee/spec, the naming will be good again! :)

One benefit of moving EE-specific specs to ee/spec

That being said, I understand this is not the current approach so we cannot resolve this for now. :)

Let's hold off on doing anything more here until we reach consensus in https://gitlab.com/gitlab-org/gitlab-ee/issues/2902 :)

@rymai Oh, that one, moving it to spec/ee/spec/lib/gitlab/visibility_level_spec.rb would also work, too! :P

I'll move all other discussions over to the issue.

added 17 commits

• 01ee3791...3fc92c79 - 10 commits from branch master
• 86fd8034 - Move EE-specific files to a standalone directory
• 76db6519 - Also move models/concerns and lib
• e8af6817 - Move views
• 7a26cf1d - Move spec files
• f4832119 - Move the new view files
• 5d924e53 - Move javascript files
• ccf288f1 - Move this spec to the right location and enable rubocop

Compare with previous version

@DouweM We're unlikely to reach a consensus on #2902 (closed) (and we shouldn't try to reach consensus anyways) so I'd be in favor of trying it and see. Also, as @vsizov said, this is a reversible decision.

I'll rebase again, but we should rebase again after https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2554 is merged because that one is more important and should go in first.

added 301 commits

• ccf288f1...767d0950 - 300 commits from branch master
• 7f2ec593 - Merge remote-tracking branch 'ee/master' into 2902-standalone-ee-dir

Compare with previous version

@rymai Sure, let's try it.

added 341 commits

• 7f2ec593...dd178124 - 340 commits from branch master
• 9213d306 - Merge remote-tracking branch 'ee/master' into 2902-standalone-ee-dir

Compare with previous version

added 1 commit

• 16ff81b9 - Move the new files

Compare with previous version

added 1 commit

• ce82d2ca - Fix json schema definition

Compare with previous version

@rymai Anything else you think we need? Should we update the doc somehow?

@mikegreiling Should we update http://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html#modules-imports-and-exports ? How should we phrase it?

changed milestone to %9.5

added 9 commits

• ce82d2ca...4afbcb28 - 8 commits from branch master
• bcc059c4 - Merge remote-tracking branch 'ee/master' into 2902-standalone-ee-dir

Compare with previous version

Let's change that part like so:

Relative paths: when importing a module in the same directory, a child directory, or an immediate parent directory prefer relative paths. When importing a module which is two or more levels up, prefer either ~/ or ee/.

In app/assets/javascripts/my-feature/subdir:

// bad
import Foo from '~/my-feature/foo';
import Bar from '~/my-feature/subdir/bar';
import Bin from '~/my-feature/subdir/lib/bin';

// good
import Foo from '../foo';
import Bar from './bar';
import Bin from './lib/bin';

In spec/javascripts:

// bad
import Foo from '../../app/assets/javascripts/my-feature/foo';

// good
import Foo from '~/my-feature/foo';

When referencing an EE component:

// bad
import Foo from '../../../../../ee/app/assets/javascripts/my-feature/ee-foo';

// good
import Foo from 'ee/my-feature/foo';

mentioned in commit 69d6e911

added 1 commit

• 69d6e911 - Update JavaScript style guide according to:

Compare with previous version

@mikegreiling Thanks, looks good to me! Pushed that one

added 1 commit

• 0be3a416 - Update EE doc about where to place the files

Compare with previous version

marked the checklist item Documentation created/updated as completed

@rymai I updated the doc a bit and it might conflict with https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2248

What do you think? Should we update the doc more in this MR?

assigned to @rymai

resolved all discussions

mentioned in issue #3097 (closed)

What do you think? Should we update the doc more in this MR?

@godfat Let's merge this one first since it might conflict if we wait more.

approved this merge request

merged

mentioned in commit c7837b50

mentioned in commit 19352a4d

mentioned in issue #3148

mentioned in commit 35a283d6