From 08eaa59359ff1afc8ecac5b942d0f5b355cb8df4 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Fri, 3 Nov 2017 09:44:00 +0100 Subject: Added + Updated Document for Vue Component --- doc/development/fe_guide/icons.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'doc/development') diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index cef62618a3c..382c7311905 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -4,15 +4,17 @@ We are using SVG Icons in GitLab with a SVG Sprite, due to this the icons are on ### Usage in HAML/Rails -To use a sprite Icon in HAML or Rails we use a specific helper function : +To use a sprite Icon in HAML or Rails we use a specific helper function : `sprite_icon(icon_name, size: nil, css_class: '')` -**icon_name** Use the icon_name that you can find in the SVG Sprite (Overview is available under `/assets/sprite.symbol.html`). +**icon_name** Use the icon_name that you can find in the SVG Sprite ([Overview is available here](http://gitlab-org.gitlab.io/gitlab-svgs/)`). + **size (optional)** Use one of the following sizes : 16,24,32,48,72 (this will be translated into a `s16` class) + **css_class (optional)** If you want to add additional css classes -**Example** +**Example** `= sprite_icon('issues', size: 72, css_class: 'icon-danger')` @@ -20,9 +22,27 @@ To use a sprite Icon in HAML or Rails we use a specific helper function : `` +### Usage in Vue + +We have a special Vue component for our sprite icons in `\vue_shared\components\icon.vue`. + +Sample usage : + +`` + +**name** Name of the Icon in the SVG Sprite ([Overview is available here](http://gitlab-org.gitlab.io/gitlab-svgs/)`). + +**size (optional)** Number value for the size which is then mapped to a specific CSS class (Available Sizes: 8,12,16,18,24,32,48,72 are mapped to `sXX` css classes) + +**css-classes (optional)** Additional CSS Classes to add to the svg tag. + ### Usage in HTML/JS -Please use the following function inside JS to render an icon : +Please use the following function inside JS to render an icon : `gl.utils.spriteIcon(iconName)` ## Adding a new icon to the sprite -- cgit v1.2.1 From 245d0daddef23427d9b1aa206dbedf5b9cc89b34 Mon Sep 17 00:00:00 2001 From: Tim Zallmann Date: Tue, 14 Nov 2017 20:08:51 +0100 Subject: Added info about tracking --- doc/development/fe_guide/icons.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index 382c7311905..b288ee95722 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -49,7 +49,7 @@ Please use the following function inside JS to render an icon : All Icons and Illustrations are managed in the [gitlab-svgs](https://gitlab.com/gitlab-org/gitlab-svgs) repository which is added as a dev-dependency. -To upgrade to a new SVG Sprite version run `yarn upgrade @gitlab-org/gitlab-svgs` and then run `yarn run svg`. This task will copy the svg sprite and all illustrations in the correct folders. +To upgrade to a new SVG Sprite version run `yarn upgrade @gitlab-org/gitlab-svgs` and then run `yarn run svg`. This task will copy the svg sprite and all illustrations in the correct folders. The updated files should be tracked in Git as those are referenced. # SVG Illustrations -- cgit v1.2.1 From 8922b7b781f241d9afd77f15fad0cfcab14d5205 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Thu, 16 Nov 2017 18:03:21 +0000 Subject: Update database_debugging.md --- doc/development/database_debugging.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'doc/development') diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index 4acfbef3020..e1c785d85fa 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -9,7 +9,6 @@ An easy first step is to search for your error in Slack or google "GitLab Date: Thu, 16 Nov 2017 19:17:31 +0000 Subject: Update database_debugging.md --- doc/development/database_debugging.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'doc/development') diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index e1c785d85fa..50eb8005b44 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -9,17 +9,24 @@ An easy first step is to search for your error in Slack or google "GitLab Date: Fri, 17 Nov 2017 10:57:57 +0000 Subject: Add dropdowns documentation --- doc/development/fe_guide/dropdowns.md | 38 +++++++++++++++++++++++++++++++++++ doc/development/fe_guide/index.md | 2 ++ 2 files changed, 40 insertions(+) create mode 100644 doc/development/fe_guide/dropdowns.md (limited to 'doc/development') diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md new file mode 100644 index 00000000000..e1660ac5caa --- /dev/null +++ b/doc/development/fe_guide/dropdowns.md @@ -0,0 +1,38 @@ +# Dropdowns + + +## How to style a bootstrap dropdown +1. Use the HTML structure provided by the [docs][bootstrap-dropdowns] +1. Add a specific class to the top level `.dropdown` element + + + ```Haml + .dropdown.my-dropdown + %button{ type: 'button', data: { toggle: 'dropdown' }, 'aria-haspopup': true, 'aria-expanded': false } + %span.dropdown-toggle-text + Toggle Dropdown + = icon('chevron-down') + + %ul.dropdown-menu + %li + %a + item! + ``` + + Or use the helpers + ```Haml + .dropdown.my-dropdown + = dropdown_toggle('Toogle!', { toggle: 'dropdown' }) + = dropdown_content + %li + %a + item! + ``` + +1. Include the mixin in CSS + + ```SCSS + @include new-style-dropdown('.my-dropdown '); + ``` + +[bootstrap-dropdowns]: https://getbootstrap.com/docs/3.3/javascript/#dropdowns diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 8f956681693..73a03c07812 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -77,6 +77,8 @@ Vue resource specific practices and gotchas. ## [Icons](icons.md) How we use SVG for our Icons. +## [Dropdowns](dropdowns.md) +How we use dropdowns. --- ## Style Guides -- cgit v1.2.1 From 4d367dd40037500fd7b96059fa300cbe2ce28e85 Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 17 Nov 2017 16:02:10 +0000 Subject: Add computed update docs for update_column_in_batches --- doc/development/migration_style_guide.md | 38 +++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 9b8ab5da74e..a235dd74909 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -198,7 +198,43 @@ end Keep in mind that this operation can easily take 10-15 minutes to complete on larger installations (e.g. GitLab.com). As a result you should only add default -values if absolutely necessary. +values if absolutely necessary. There is a RuboCop cop that will fail if this +method is used on some tables that are very large on GitLab.com, which would +cause other issues. + +## Updating an existing column + +To update an existing column to a particular value, you can use +`update_column_in_batches` (`add_column_with_default` uses this internally to +fill in the default value). This will split the updates into batches, so we +don't update too many rows at in a single statement. + +This updates the column `foo` in the `projects` table to 10, where `some_column` +is `'hello'`: + +```ruby +update_column_in_batches(:projects, :foo, 10) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +To perform a computed update, the value can be wrapped in `Arel.sql`, so Arel +treats it as an SQL literal. The below example is the same as the one above, but +the value is set to the product of the `bar` and `baz` columns: + +```ruby +update_value = Arel.sql('bar * baz') + +update_column_in_batches(:projects, :foo, update_value) do |table, query| + query.where(table[:some_column].eq('hello')) +end +``` + +Like `add_column_with_default`, there is a RuboCop cop to detect usage of this +on large tables. In the case of `update_column_in_batches`, it may be acceptable +to run on a large table, as long as it is only updating a small subset of the +rows in the table, but do not ignore that without validating on the GitLab.com +staging environment - or asking someone else to do so for you - beforehand. ## Integer column type -- cgit v1.2.1 From 9400ed3b3e3bf1c1ce20b9748f4b6e4c0e5b5db7 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 20 Nov 2017 09:57:08 +0000 Subject: Use axios instead of vue resource - step 1 --- doc/development/fe_guide/axios.md | 68 ++++++++++++++++++++++++++++++ doc/development/fe_guide/index.md | 4 +- doc/development/fe_guide/vue.md | 70 ++++++------------------------- doc/development/fe_guide/vue_resource.md | 72 -------------------------------- 4 files changed, 83 insertions(+), 131 deletions(-) create mode 100644 doc/development/fe_guide/axios.md delete mode 100644 doc/development/fe_guide/vue_resource.md (limited to 'doc/development') diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md new file mode 100644 index 00000000000..962fe3dcec9 --- /dev/null +++ b/doc/development/fe_guide/axios.md @@ -0,0 +1,68 @@ +# Axios +We use [axios][axios] to communicate with the server in Vue applications and most new code. + +In order to guarantee all defaults are set you *should not use `axios` directly*, you should import `axios` from `axios_utils`. + +## CSRF token +All our request require a CSRF token. +To guarantee this token is set, we are importing [axios][axios], setting the token, and exporting `axios` . + +This exported module should be used instead of directly using `axios` to ensure the token is set. + +## Usage +```javascript + import axios from '~/lib/utils/axios_utils'; + + axios.get(url) + .then((response) => { + // `data` is the response that was provided by the server + const data = response.data; + + // `headers` the headers that the server responded with + // All header names are lower cased + const paginationData = response.headers; + }) + .catch(() => { + //handle the error + }); +``` + +## Mock axios response on tests + +To help us mock the responses we need we use [axios-mock-adapter][axios-mock-adapter] + + +```javascript + import axios from '~/lib/utils/axios_utils'; + import MockAdapter from 'axios-mock-adapter'; + + let mock; + beforeEach(() => { + // This sets the mock adapter on the default instance + mock = new MockAdapter(axios); + // Mock any GET request to /users + // arguments for reply are (status, data, headers) + mock.onGet('/users').reply(200, { + users: [ + { id: 1, name: 'John Smith' } + ] + }); + }); + + afterEach(() => { + mock.reset(); + }); +``` + +### Mock poll requests on tests with axios + +Because polling function requires an header object, we need to always include an object as the third argument: + +```javascript + mock.onGet('/users').reply(200, { foo: 'bar' }, {}); +``` + +[axios]: https://github.com/axios/axios +[axios-instance]: #creating-an-instance +[axios-interceptors]: https://github.com/axios/axios#interceptors +[axios-mock-adapter]: https://github.com/ctimmerm/axios-mock-adapter diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 73a03c07812..72cb557d054 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -71,8 +71,8 @@ Vue specific design patterns and practices. --- -## [Vue Resource](vue_resource.md) -Vue resource specific practices and gotchas. +## [Axios](axios.md) +Axios specific practices and gotchas. ## [Icons](icons.md) How we use SVG for our Icons. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index f88f0753687..6e9f18dd1c3 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -178,16 +178,13 @@ itself, please read this guide: [State Management][state-management] The Service is a class used only to communicate with the server. It does not store or manipulate any data. It is not aware of the store or the components. -We use [vue-resource][vue-resource-repo] to communicate with the server. -Refer to [vue resource](vue_resource.md) for more details. +We use [axios][axios] to communicate with the server. +Refer to [axios](axios.md) for more details. -Vue Resource should only be imported in the service file. +Axios instance should only be imported in the service file. ```javascript - import Vue from 'vue'; - import VueResource from 'vue-resource'; - - Vue.use(VueResource); + import axios from 'javascripts/lib/utils/axios_utils'; ``` ### End Result @@ -230,15 +227,14 @@ export default class Store { } // service.js -import Vue from 'vue'; -import VueResource from 'vue-resource'; -import 'vue_shared/vue_resource_interceptor'; - -Vue.use(VueResource); +import axios from 'javascripts/lib/utils/axios_utils' export default class Service { constructor(options) { - this.todos = Vue.resource(endpoint.todosEndpoint); + this.todos = axios.create({ + baseURL: endpoint.todosEndpoint + }); + } getTodos() { @@ -477,50 +473,8 @@ The main return value of a Vue component is the rendered output. In order to tes need to test the rendered output. [Vue][vue-test] guide's to unit test show us exactly that: ### Stubbing API responses -[Vue Resource Interceptors][vue-resource-interceptor] allow us to add a interceptor with -the response we need: - - ```javascript - // Mock the service to return data - const interceptor = (request, next) => { - next(request.respondWith(JSON.stringify([{ - title: 'This is a todo', - body: 'This is the text' - }]), { - status: 200, - })); - }; +Refer to [mock axios](axios.md#mock-axios-response-on-tests) - beforeEach(() => { - Vue.http.interceptors.push(interceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor); - }); - - it('should do something', (done) => { - setTimeout(() => { - // Test received data - done(); - }, 0); - }); - ``` - -1. Headers interceptor -Refer to [this section](vue.md#headers) - -1. Use `$.mount()` to mount the component - -```javascript -// bad -new Component({ - el: document.createElement('div') -}); - -// good -new Component().$mount(); -``` ## Vuex To manage the state of an application you may use [Vuex][vuex-docs]. @@ -721,7 +675,6 @@ describe('component', () => { [component-system]: https://vuejs.org/v2/guide/#Composing-with-Components [state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch [one-way-data-flow]: https://vuejs.org/v2/guide/components.html#One-Way-Data-Flow -[vue-resource-interceptor]: https://github.com/pagekit/vue-resource/blob/develop/docs/http.md#interceptors [vue-test]: https://vuejs.org/v2/guide/unit-testing.html [issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 [flux]: https://facebook.github.io/flux @@ -729,3 +682,6 @@ describe('component', () => { [vuex-structure]: https://vuex.vuejs.org/en/structure.html [vuex-mutations]: https://vuex.vuejs.org/en/mutations.html [vuex-testing]: https://vuex.vuejs.org/en/testing.html +[axios]: https://github.com/axios/axios +[axios-interceptors]: https://github.com/axios/axios#interceptors + diff --git a/doc/development/fe_guide/vue_resource.md b/doc/development/fe_guide/vue_resource.md deleted file mode 100644 index c376c5c32bf..00000000000 --- a/doc/development/fe_guide/vue_resource.md +++ /dev/null @@ -1,72 +0,0 @@ -# Vue Resouce -In Vue applications we use [vue-resource][vue-resource-repo] to communicate with the server. - -## HTTP Status Codes - -### `.json()` -When making a request to the server, you will most likely need to access the body of the response. -Use `.json()` to convert. Because `.json()` returns a Promise the follwoing structure should be used: - - ```javascript - service.get('url') - .then(resp => resp.json()) - .then((data) => { - this.store.storeData(data); - }) - .catch(() => new Flash('Something went wrong')); - ``` - - -When using `Poll` (`app/assets/javascripts/lib/utils/poll.js`), the `successCallback` needs to handle `.json()` as a Promise: - ```javascript - successCallback: (response) => { - return response.json().then((data) => { - // handle the response - }); - } - ``` - -### 204 -Some endpoints - usually `delete` endpoints - return `204` as the success response. -When handling `204 - No Content` responses, we cannot use `.json()` since it tries to parse the non-existant body content. - -When handling `204` responses, do not use `.json`, otherwise the promise will throw an error and will enter the `catch` statement: - -```javascript - Vue.http.delete('path') - .then(() => { - // success! - }) - .catch(() => { - // handle error - }) -``` - -## Headers -Headers are being parsed into a plain object in an interceptor. -In Vue-resource 1.x `headers` object was changed into an `Headers` object. In order to not change all old code, an interceptor was added. - -If you need to write a unit test that takes the headers in consideration, you need to include an interceptor to parse the headers after your test interceptor. -You can see an example in `spec/javascripts/environments/environment_spec.js`: - ```javascript - import { headersInterceptor } from './helpers/vue_resource_helper'; - - beforeEach(() => { - Vue.http.interceptors.push(myInterceptor); - Vue.http.interceptors.push(headersInterceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, myInterceptor); - Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor); - }); - ``` - -## CSRF token -We use a Vue Resource interceptor to manage the CSRF token. -`app/assets/javascripts/vue_shared/vue_resource_interceptor.js` holds all our common interceptors. -Note: You don't need to load `app/assets/javascripts/vue_shared/vue_resource_interceptor.js` -since it's already being loaded by `common_vue.js`. - - -[vue-resource-repo]: https://github.com/pagekit/vue-resource -- cgit v1.2.1 From 52b6cbcb9df7e2c968da3e9a9779500cacfac465 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 20 Nov 2017 09:18:15 -0800 Subject: Add QUERY_RECORDER_DEBUG environment variable to improve performance debugging --- doc/development/query_recorder.md | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'doc/development') diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index e0127aaed4c..12e90101139 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -22,6 +22,52 @@ As an example you might create 5 issues in between counts, which would cause the > **Note:** In some cases the query count might change slightly between runs for unrelated reasons. In this case you might need to test `exceed_query_limit(control_count + acceptable_change)`, but this should be avoided if possible. +## Finding the source of the query + +It may be useful to identify the source of the queries by looking at the call backtrace. +To enable this, run the specs with the `QUERY_RECORDER_DEBUG` environment variable set. For example: + +``` +QUERY_RECORDER_DEBUG=1 bundle exec rspec spec/requests/api/projects_spec.rb +``` + +This will log calls to QueryRecorder into the `test.log`. For example: + +``` +QueryRecorder SQL: SELECT COUNT(*) FROM "issues" WHERE "issues"."deleted_at" IS NULL AND "issues"."project_id" = $1 AND ("issues"."state" IN ('opened')) AND "issues"."confidential" = $2 + --> /home/user/gitlab/gdk/gitlab/spec/support/query_recorder.rb:19:in `callback' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:127:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `block in finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `each' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/fanout.rb:46:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/instrumenter.rb:36:in `finish' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications/instrumenter.rb:25:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract_adapter.rb:478:in `log' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:601:in `exec_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql_adapter.rb:585:in `execute_and_clear' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/postgresql/database_statements.rb:160:in `exec_query' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/database_statements.rb:356:in `select' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/database_statements.rb:32:in `select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:68:in `block in select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:83:in `cache_sql' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/connection_adapters/abstract/query_cache.rb:68:in `select_all' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:270:in `execute_simple_calculation' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:227:in `perform_calculation' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:133:in `calculate' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activerecord-4.2.8/lib/active_record/relation/calculations.rb:48:in `count' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:20:in `uncached_count' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:12:in `block in count' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:299:in `block in fetch' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:585:in `block in save_block_result_to_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:547:in `block in instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/notifications.rb:166:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:547:in `instrument' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:584:in `save_block_result_to_cache' + --> /home/user/.rbenv/versions/2.3.5/lib/ruby/gems/2.3.0/gems/activesupport-4.2.8/lib/active_support/cache.rb:299:in `fetch' + --> /home/user/gitlab/gdk/gitlab/app/services/base_count_service.rb:12:in `count' + --> /home/user/gitlab/gdk/gitlab/app/models/project.rb:1296:in `open_issues_count' +``` + ## See also - [Bullet](profiling.md#Bullet) For finding `N+1` query problems -- cgit v1.2.1 From 3f39d787953263d8102437a1f9838548d94eac43 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 23 Nov 2017 11:22:33 +0100 Subject: Fix the redirect location wording Closes https://gitlab.com/gitlab-com/gitlab-docs/issues/142 --- doc/development/doc_styleguide.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'doc/development') diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index 0e4ffbd7910..aaa7032cadb 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -303,10 +303,10 @@ GitLab.com or http://docs.gitlab.com. Make sure this is discussed with the Documentation team beforehand. If you indeed need to change a document's location, do NOT remove the old -document, but rather put a text in it that points to the new location, like: +document, but rather replace all of its contents with a new line: ``` -This document was moved to [path/to/new_doc.md](path/to/new_doc.md). +This document was moved to [another location](path/to/new_doc.md). ``` where `path/to/new_doc.md` is the relative path to the root directory `doc/`. @@ -320,7 +320,7 @@ For example, if you were to move `doc/workflow/lfs/lfs_administration.md` to 1. Replace the contents of `doc/workflow/lfs/lfs_administration.md` with: ``` - This document was moved to [administration/lfs.md](../../administration/lfs.md). + This document was moved to [another location](../../administration/lfs.md). ``` 1. Find and replace any occurrences of the old location with the new one. -- cgit v1.2.1 From 07e42c5c425b5a781398fea2aaf7eeb8a825724f Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 23 Nov 2017 13:17:10 +0100 Subject: Clarify the docs review process and mention the supported repos --- doc/development/writing_documentation.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/writing_documentation.md b/doc/development/writing_documentation.md index 68ba3dd2da3..b6def7ef541 100644 --- a/doc/development/writing_documentation.md +++ b/doc/development/writing_documentation.md @@ -152,12 +152,23 @@ CE and EE. ## Previewing the changes live If you want to preview the doc changes of your merge request live, you can use -the manual `review-docs-deploy` job in your merge request. +the manual `review-docs-deploy` job in your merge request. You will need at +least Master permissions to be able to run it and is currently enabled for the +following projects: + +- https://gitlab.com/gitlab-org/gitlab-ce +- https://gitlab.com/gitlab-org/gitlab-ee + +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. TIP: **Tip:** If your branch contains only documentation changes, you can use [special branch names](#testing) to avoid long running pipelines. +In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy` job. Hit the play button for the job to start. + ![Manual trigger a docs build](img/manual_build_docs.png) This job will: -- cgit v1.2.1 From 9cb38f0433930f85964ab3c3f07d677676fa265b Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Fri, 24 Nov 2017 09:55:23 +0000 Subject: Fix instructions for creating project templates Sidekiq has to be running too. --- doc/development/rake_tasks.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index 4773b6773e8..ceff57276d2 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -163,7 +163,7 @@ Starting a project from a template needs this project to be exported. On a up to date master branch with run: ``` -gdk run db +gdk run # In a new terminal window bundle exec rake gitlab:update_project_templates git checkout -b update-project-templates -- cgit v1.2.1 From 9295c827b85ef76f40dd23b2e00909df94eea093 Mon Sep 17 00:00:00 2001 From: digitalMoksha Date: Wed, 29 Nov 2017 10:44:33 +0100 Subject: fix link that was linking to `html` instead of `md` (to be consistent) --- doc/development/what_requires_downtime.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index c4830322fa8..05e0a64af18 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -37,7 +37,7 @@ when using the migration helper method `Gitlab::Database::MigrationHelpers#add_column_with_default`. This method works similar to `add_column` except it updates existing rows in batches without blocking access to the table being modified. See ["Adding Columns With Default -Values"](migration_style_guide.html#adding-columns-with-default-values) for more +Values"](migration_style_guide.md#adding-columns-with-default-values) for more information on how to use this method. ## Dropping Columns -- cgit v1.2.1 From 0f99e9a50b3faf1da47d37a0642d22e636498a80 Mon Sep 17 00:00:00 2001 From: Takuya Noguchi Date: Sun, 3 Dec 2017 13:03:45 +0900 Subject: Fix typo in docs about Elasticsearch --- doc/development/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'doc/development') diff --git a/doc/development/changelog.md b/doc/development/changelog.md index f869938fe11..48cffc0dd18 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -80,7 +80,7 @@ changes. The first example focuses on _how_ we fixed something, not on _what_ it fixes. The rewritten version clearly describes the _end benefit_ to the user (fewer 500 -errors), and _when_ (searching commits with ElasticSearch). +errors), and _when_ (searching commits with Elasticsearch). Use your best judgement and try to put yourself in the mindset of someone reading the compiled changelog. Does this entry add value? Does it offer context -- cgit v1.2.1 From 6cf76d652d0ae0b77b1250a81aa24e3ce164ccbc Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Mon, 4 Dec 2017 11:00:52 +0000 Subject: Add underline hover state to all links --- doc/development/doc_styleguide.md | 6 ------ 1 file changed, 6 deletions(-) (limited to 'doc/development') diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index aaa7032cadb..db13e0e6249 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -170,12 +170,6 @@ You can combine one or more of the following: = link_to 'Help page', help_page_path('user/permissions'), class: 'btn btn-info' ``` -1. **Underlining a link.** - - ```haml - = link_to 'Help page', help_page_path('user/permissions'), class: 'underlined-link' - ``` - 1. **Using links inline of some text.** ```haml -- cgit v1.2.1 From 0b15570e497d3c5c515be59a43b686087b985f5c Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 28 Nov 2017 17:08:30 +0100 Subject: Add ApplicationWorker and make every worker include it --- doc/development/sidekiq_style_guide.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'doc/development') diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 1e9fdbc65e2..a120f3e4771 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -18,8 +18,7 @@ include the `DedicatedSidekiqQueue` concern as follows: ```ruby class ProcessSomethingWorker - include Sidekiq::Worker - include DedicatedSidekiqQueue + include ApplicationWorker end ``` -- cgit v1.2.1 From a5c3f1c8ff7da20183b172b2b0693a6010c9e86d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 30 Nov 2017 16:28:09 +0100 Subject: Update docs --- doc/development/sidekiq_style_guide.md | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'doc/development') diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index a120f3e4771..085fb8f902c 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -3,6 +3,12 @@ This document outlines various guidelines that should be followed when adding or modifying Sidekiq workers. +## ApplicationWorker + +All workers should include `ApplicationWorker` instead of `Sidekiq::Worker`, +which adds some convenience methods and automatically sets the queue based on +the worker's name. + ## Default Queue Use of the "default" queue is not allowed. Every worker should use a queue that @@ -13,18 +19,10 @@ A list of all available queues can be found in `config/sidekiq_queues.yml`. ## Dedicated Queues -Most workers should use their own queue. To ease this process a worker can -include the `DedicatedSidekiqQueue` concern as follows: - -```ruby -class ProcessSomethingWorker - include ApplicationWorker -end -``` - -This will set the queue name based on the class' name, minus the `Worker` -suffix. In the above example this would lead to the queue being -`process_something`. +Most workers should use their own queue, which is automatically set based on the +worker class name. For a worker named `ProcessSomethingWorker`, the queue name +would be `process_something`. If you're not sure what a worker's queue name is, +you can find it using `SomeWorker.queue`. In some cases multiple workers do use the same queue. For example, the various workers for updating CI pipelines all use the `pipeline` queue. Adding workers @@ -38,7 +36,7 @@ tests should be placed in `spec/workers`. ## Removing or renaming queues -Try to avoid renaming or removing queues in minor and patch releases. -During online update instance can have pending jobs and removing the queue can -lead to those jobs being stuck forever. If you can't write migration for those -Sidekiq jobs, please consider doing rename or remove queue in major release only. \ No newline at end of file +Try to avoid renaming or removing queues in minor and patch releases. +During online update instance can have pending jobs and removing the queue can +lead to those jobs being stuck forever. If you can't write migration for those +Sidekiq jobs, please consider doing rename or remove queue in major release only. -- cgit v1.2.1 From 1e6ca3c41ead23c5e433460c8c807ea73d9ec0ef Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 29 Nov 2017 16:30:17 +0100 Subject: Consistently schedule Sidekiq jobs --- doc/development/background_migrations.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'doc/development') diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 5452b0e7a2f..fd2b9d0e908 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -68,10 +68,10 @@ BackgroundMigrationWorker.perform_async('BackgroundMigrationClassName', [arg1, a ``` Usually it's better to enqueue jobs in bulk, for this you can use -`BackgroundMigrationWorker.perform_bulk`: +`BackgroundMigrationWorker.bulk_perform_async`: ```ruby -BackgroundMigrationWorker.perform_bulk( +BackgroundMigrationWorker.bulk_perform_async( [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] ) @@ -85,13 +85,13 @@ updates. Removals in turn can be handled by simply defining foreign keys with cascading deletes. If you would like to schedule jobs in bulk with a delay, you can use -`BackgroundMigrationWorker.perform_bulk_in`: +`BackgroundMigrationWorker.bulk_perform_in`: ```ruby jobs = [['BackgroundMigrationClassName', [1]], ['BackgroundMigrationClassName', [2]]] -BackgroundMigrationWorker.perform_bulk_in(5.minutes, jobs) +BackgroundMigrationWorker.bulk_perform_in(5.minutes, jobs) ``` ## Cleaning Up @@ -201,7 +201,7 @@ class ScheduleExtractServicesUrl < ActiveRecord::Migration ['ExtractServicesUrl', [id]] end - BackgroundMigrationWorker.perform_bulk(jobs) + BackgroundMigrationWorker.bulk_perform_async(jobs) end end -- cgit v1.2.1