diff options
| -rw-r--r-- | app/assets/javascripts/boards/components/modal/index.js | 4 | ||||
| -rw-r--r-- | app/assets/javascripts/pipelines/components/graph/graph_component.vue | 4 | ||||
| -rw-r--r-- | app/assets/javascripts/vue_merge_request_widget/dependencies.js | 2 | ||||
| -rw-r--r-- | config/application.rb | 4 | ||||
| -rw-r--r-- | doc/development/fe_guide/style_guide_js.md | 54 | ||||
| -rw-r--r-- | 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 |