From b7547ade672fdcb9315ae9821c1eca195d87022a Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 4 Aug 2017 21:45:14 +0800
Subject: [PATCH] Backport to CE for:
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2483
---
.../boards/components/modal/index.js | 4 +-
.../components/graph/graph_component.vue | 4 +-
.../vue_merge_request_widget/dependencies.js | 2 +-
config/application.rb | 4 +-
doc/development/fe_guide/style_guide_js.md | 54 ++++++++++++-------
spec/support/api/schema_matcher.rb | 18 ++++---
6 files changed, 53 insertions(+), 33 deletions(-)
diff --git a/app/assets/javascripts/boards/components/modal/index.js b/app/assets/javascripts/boards/components/modal/index.js
index 1d36519c75c..96af69e7a36 100644
--- a/app/assets/javascripts/boards/components/modal/index.js
+++ b/app/assets/javascripts/boards/components/modal/index.js
@@ -1,8 +1,8 @@
/* global ListIssue */
import Vue from 'vue';
-import queryData from '../../utils/query_data';
-import loadingIcon from '../../../vue_shared/components/loading_icon.vue';
+import queryData from '~/boards/utils/query_data';
+import loadingIcon from '~/vue_shared/components/loading_icon.vue';
import './header';
import './list';
import './footer';
diff --git a/app/assets/javascripts/pipelines/components/graph/graph_component.vue b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
index 77cbaeb43ef..66bc1d1979c 100644
--- a/app/assets/javascripts/pipelines/components/graph/graph_component.vue
+++ b/app/assets/javascripts/pipelines/components/graph/graph_component.vue
@@ -1,7 +1,7 @@
<script>
+ import loadingIcon from '~/vue_shared/components/loading_icon.vue';
+ import '~/flash';
import stageColumnComponent from './stage_column_component.vue';
- import loadingIcon from '../../../vue_shared/components/loading_icon.vue';
- import '../../../flash';
export default {
props: {
diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
index fe5e1bbb55c..546a3f625c7 100644
--- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js
+++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
@@ -1,7 +1,7 @@
/**
* This file is the centerpiece of an attempt to reduce potential conflicts
* between the CE and EE versions of the MR widget. EE additions to the MR widget should
- * be contained in the ./vue_merge_request_widget/ee directory, and should **extend**
+ * be contained in the ee/vue_merge_request_widget directory, and should **extend**
* rather than mutate CE MR Widget code.
*
* This file should be the only source of conflicts between EE and CE. EE-only components should
diff --git a/config/application.rb b/config/application.rb
index 1c13cc81270..f7145566262 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -23,13 +23,13 @@ module Gitlab
# https://github.com/rails/rails/blob/v4.2.6/railties/lib/rails/engine.rb#L687
# This is a nice reference article on autoloading/eager loading:
# http://blog.arkency.com/2014/11/dont-forget-about-eager-load-when-extending-autoload
- config.eager_load_paths.push(*%W(#{config.root}/lib
+ config.eager_load_paths.push(*%W[#{config.root}/lib
#{config.root}/app/models/hooks
#{config.root}/app/models/members
#{config.root}/app/models/project_services
#{config.root}/app/workers/concerns
#{config.root}/app/services/concerns
- #{config.root}/app/finders/concerns))
+ #{config.root}/app/finders/concerns])
config.generators.templates.push("#{config.root}/generator_templates")
diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md
index 149a0159680..6ade3231fac 100644
--- a/doc/development/fe_guide/style_guide_js.md
+++ b/doc/development/fe_guide/style_guide_js.md
@@ -11,7 +11,7 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns.
#### ESlint
-1. **Never** disable eslint rules unless you have a good reason.
+1. **Never** disable eslint rules unless you have a good reason.
You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */`
at the top, but legacy files are a special case. Any time you develop a new feature or
refactor an existing one, you should abide by the eslint rules.
@@ -100,26 +100,44 @@ followed by any global declarations, then a blank newline prior to any imports o
export default Foo;
```
-1. Relative paths: Unless you are writing a test, always reference other scripts using
-relative paths instead of `~`
- * In **app/assets/javascripts**:
+1. 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/`
+.
- ```javascript
- // bad
- import Foo from '~/foo'
+In **app/assets/javascripts/my-feature/subdir**:
- // good
- import Foo from '../foo';
- ```
- * In **spec/javascripts**:
+``` javascript
+// bad
+import Foo from '~/my-feature/foo';
+import Bar from '~/my-feature/subdir/bar';
+import Bin from '~/my-feature/subdir/lib/bin';
- ```javascript
- // bad
- import Foo from '../../app/assets/javascripts/foo'
+// good
+import Foo from '../foo';
+import Bar from './bar';
+import Bin from './lib/bin';
+```
- // good
- import Foo from '~/foo';
- ```
+In **spec/javascripts**:
+
+``` javascript
+// bad
+import Foo from '../../app/assets/javascripts/my-feature/foo';
+
+// good
+import Foo from '~/my-feature/foo';
+```
+
+When referencing an **EE component**:
+
+``` javascript
+// bad
+import Foo from '../../../../../ee/app/assets/javascripts/my-feature/ee-foo';
+
+// good
+import Foo from 'ee/my-feature/foo';
+```
1. Avoid using IIFE. Although we have a lot of examples of files which wrap their
contents in IIFEs (immediately-invoked function expressions),
@@ -465,7 +483,7 @@ A forEach will cause side effects, it will be mutating the array being iterated.
#### Vue and Boostrap
1. Tooltips: Do not rely on `has-tooltip` class name for Vue components
- ``` javascript
+ ```javascript
// bad
<span
class="has-tooltip"
diff --git a/spec/support/api/schema_matcher.rb b/spec/support/api/schema_matcher.rb
index 67599f77adb..6591d56e473 100644
--- a/spec/support/api/schema_matcher.rb
+++ b/spec/support/api/schema_matcher.rb
@@ -1,23 +1,25 @@
-def schema_path(schema)
- schema_directory = "#{Dir.pwd}/spec/fixtures/api/schemas"
- "#{schema_directory}/#{schema}.json"
+module SchemaPath
+ def self.expand(schema, dir = '')
+ Rails.root.join('spec', dir, "fixtures/api/schemas/#{schema}.json").to_s
+ end
end
-RSpec::Matchers.define :match_response_schema do |schema, **options|
+RSpec::Matchers.define :match_response_schema do |schema, dir: '', **options|
match do |response|
- @errors = JSON::Validator.fully_validate(schema_path(schema), response.body, options)
+ @errors = JSON::Validator.fully_validate(
+ SchemaPath.expand(schema, dir), response.body, options)
@errors.empty?
end
failure_message do |response|
- "didn't match the schema defined by #{schema_path(schema)}" \
+ "didn't match the schema defined by #{SchemaPath.expand(schema, dir)}" \
" The validation errors were:\n#{@errors.join("\n")}"
end
end
-RSpec::Matchers.define :match_schema do |schema, **options|
+RSpec::Matchers.define :match_schema do |schema, dir: '', **options|
match do |data|
- JSON::Validator.validate!(schema_path(schema), data, options)
+ JSON::Validator.validate!(SchemaPath.expand(schema, dir), data, options)
end
end
--
GitLab