diff options
Diffstat (limited to 'doc/development')
| -rw-r--r-- | doc/development/documentation/index.md | 39 | ||||
| -rw-r--r-- | doc/development/documentation/styleguide.md | 18 | ||||
| -rw-r--r-- | doc/development/fe_guide/vuex.md | 4 | ||||
| -rw-r--r-- | doc/development/gotchas.md | 48 | ||||
| -rw-r--r-- | doc/development/i18n/proofreader.md | 2 | ||||
| -rw-r--r-- | doc/development/query_recorder.md | 1 | ||||
| -rw-r--r-- | doc/development/utilities.md | 41 | ||||
| -rw-r--r-- | doc/development/what_requires_downtime.md | 33 |
8 files changed, 150 insertions, 36 deletions
diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 48e1685082a..f5cdd310f6f 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -322,50 +322,49 @@ to EE only. ## Previewing the changes live -To preview your changes to documentation locally, please follow -this [development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development). +NOTE: **Note:** +To preview your changes to documentation locally, follow this +[development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development). -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. You will need at -least Maintainer permissions to be able to run it and is currently enabled for the -following projects: +The live preview is currently enabled for the following projects: - https://gitlab.com/gitlab-org/gitlab-ce - https://gitlab.com/gitlab-org/gitlab-ee +- https://gitlab.com/gitlab-org/gitlab-runner -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](#branch-naming) 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. +For [docs-only changes](#branch-naming), the review app is run automatically. +For all other branches, you can use the manual `review-docs-deploy-manual` job +in your merge request. You will need at least Maintainer permissions to be able +to run it. In the mini pipeline graph, you should see an `>>` icon. Clicking on it will +reveal the `review-docs-deploy-manual` job. Hit the play button for the job to start.  -This job will: +NOTE: **Note:** +You will need to push a branch to those repositories, it doesn't work for forks. + +The `review-docs-deploy*` job will: 1. Create a new branch in the [gitlab-docs](https://gitlab.com/gitlab-com/gitlab-docs) - project named after the scheme: `preview-<branch-slug>` + project named after the scheme: `$DOCS_GITLAB_REPO_SUFFIX-$CI_ENVIRONMENT_SLUG`, + where `DOCS_GITLAB_REPO_SUFFIX` is the suffix for each product, e.g, `ce` for + CE, etc. 1. Trigger a cross project pipeline and build the docs site with your changes After a few minutes, the Review App will be deployed and you will be able to preview the changes. The docs URL can be found in two places: - In the merge request widget -- In the output of the `review-docs-deploy` job, which also includes the +- In the output of the `review-docs-deploy*` job, which also includes the triggered pipeline so that you can investigate whether something went wrong In case the Review App URL returns 404, follow these steps to debug: 1. **Did you follow the URL from the merge request widget?** If yes, then check if - the link is the same as the one in the job output. It can happen that if the - branch name slug is longer than 35 characters, it is automatically - truncated. That means that the merge request widget will not show the proper - URL due to a limitation of how `environment: url` works, but you can find the - real URL from the output of the `review-docs-deploy` job. + the link is the same as the one in the job output. 1. **Did you follow the URL from the job output?** If yes, then it means that either the site is not yet deployed or something went wrong with the remote pipeline. Give it a few minutes and it should appear online, otherwise you diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index e7ffba635c9..a315cfdc116 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -26,7 +26,11 @@ Check the GitLab handbook for the [writing styles guidelines](https://about.gitl - Jump a line between different markups (e.g., after every paragraph, header, list, etc) - Capitalize "G" and "L" in GitLab - Use sentence case for titles, headings, labels, menu items, and buttons. -- Use title case when referring to [features](https://about.gitlab.com/features/) or [products](https://about.gitlab.com/pricing/), and methods. Note that some features are also objects (e.g. "Merge Requests" and "merge requests"). E.g.: GitLab Runner, Geo, Issue Boards, Git, Prometheus, Continuous Integration. +- Use title case when referring to [features](https://about.gitlab.com/features/) or +[products](https://about.gitlab.com/pricing/) (e.g., GitLab Runner, Geo, +Issue Boards, GitLab Core, Git, Prometheus, Kubernetes, etc), and methods or methodologies +(e.g., Continuous Integration, Continuous Deployment, Scrum, Agile, etc). Note that +some features are also objects (e.g. "Merge Requests" and "merge requests"). ## Formatting @@ -72,7 +76,7 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl This is to ensure that no document with wrong heading is going live without an audit, thus preventing dead links and redirection issues when corrected -- Leave exactly one newline after a heading +- Leave exactly one new line after a heading ## Links @@ -95,6 +99,16 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl E.g., instead of writing something like `Read more about GitLab Issue Boards [here](LINK)`, write `Read more about [GitLab Issue Boards](LINK)`. +## Navigation + +To indicate the steps of navigation through the UI: + +- Use the exact word as shown in the UI, including any capital letters as-is +- Use bold text for navigation items and the char `>` as separator +(e.g., `Navigate to your project's **Settings > CI/CD**` ) +- If there are any expandable menus, make sure to mention that the user +needs to expand the tab to find the settings you're referring to + ## Images - Place images in a separate directory named `img/` in the same directory where diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 858b03c60bf..4089cd37d73 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -78,7 +78,7 @@ In this file, we will write the actions that will call the respective mutations: ```javascript import * as types from './mutation_types'; - import axios from '~/lib/utils/axios-utils'; + import axios from '~/lib/utils/axios_utils'; import createFlash from '~/flash'; export const requestUsers = ({ commit }) => commit(types.REQUEST_USERS); @@ -214,7 +214,7 @@ import { mapGetters } from 'vuex'; }; ``` -### `mutations_types.js` +### `mutation_types.js` From [vuex mutations docs][vuex-mutations]: > It is a commonly seen pattern to use constants for mutation types in various Flux implementations. This allows the code to take advantage of tooling like linters, and putting all constants in a single file allows your collaborators to get an at-a-glance view of what mutations are possible in the entire application. diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md index 5786287d00c..d25d856c3a3 100644 --- a/doc/development/gotchas.md +++ b/doc/development/gotchas.md @@ -92,6 +92,54 @@ describe API::Labels do end ``` +## Avoid using `allow_any_instance_of` in RSpec + +### Why + +* Because it is not isolated therefore it might be broken at times. +* Because it doesn't work whenever the method we want to stub was defined + in a prepended module, which is very likely the case in EE. We could see + error like this: + + 1.1) Failure/Error: allow_any_instance_of(ApplicationSetting).to receive_messages(messages) + Using `any_instance` to stub a method (elasticsearch_indexing) that has been defined on a prepended module (EE::ApplicationSetting) is not supported. + +### Alternative: `expect_next_instance_of` + +Instead of writing: + +```ruby +# Don't do this: +allow_any_instance_of(Project).to receive(:add_import_job) +``` + +We could write: + +```ruby +# Do this: +expect_next_instance_of(Project) do |project| + expect(project).to receive(:add_import_job) +end +``` + +If we also want to expect the instance was initialized with some particular +arguments, we could also pass it to `expect_next_instance_of` like: + +```ruby +# Do this: +expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service| + expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref) +end +``` + +This would expect the following: + +```ruby +# Above expects: +refresh_service = MergeRequests::RefreshService.new(project, user) +refresh_service.execute(oldrev, newrev, ref) +``` + ## Do not `rescue Exception` See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception]. diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 9a677bf09b2..9d0d7348df9 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -24,6 +24,7 @@ are very appreciative of the work done by translators and proofreaders! - Paolo Falomo - [GitLab](https://gitlab.com/paolofalomo), [Crowdin](https://crowdin.com/profile/paolo.falomo) - Japanese - Yamana Tokiuji - [GitLab](https://gitlab.com/tokiuji), [Crowdin](https://crowdin.com/profile/yamana) + - Hiroyuki Sato - [GitLab](https://gitlab.com/hiroponz), [Crowdin](https://crowdin.com/profile/hiroponz) - Korean - Chang-Ho Cha - [GitLab](https://gitlab.com/changho-cha), [Crowdin](https://crowdin.com/profile/zzazang) - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) @@ -31,6 +32,7 @@ are very appreciative of the work done by translators and proofreaders! - Filip Mech - [GitLab](https://gitlab.com/mehenz), [Crowdin](https://crowdin.com/profile/mehenz) - Portuguese, Brazilian - Paulo George Gomes Bezerra - [GitLab](https://gitlab.com/paulobezerra), [Crowdin](https://crowdin.com/profile/paulogomes.rep) + - André Gama - [GitLab](https://gitlab.com/andregamma), [Crowdin](https://crowdin.com/profile/ToeOficial) - Russian - Nikita Grylov - [GitLab](https://gitlab.com/nixel2007), [Crowdin](https://crowdin.com/profile/nixel2007) - Alexy Lustin - [GitLab](https://gitlab.com/allustin), [Crowdin](https://crowdin.com/profile/lustin) diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index 61e5e1afede..2167ed57428 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -28,6 +28,7 @@ By default, QueryRecorder will ignore cached queries in the count. However, it m all queries to avoid introducing an N+1 query that may be masked by the statement cache. To do this, pass the `skip_cached` variable to `QueryRecorder` and use the `exceed_all_query_limit` matcher: +``` it "avoids N+1 database queries" do control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { visit_some_page }.count create_list(:issue, 5) diff --git a/doc/development/utilities.md b/doc/development/utilities.md index 8f9aff1a35f..0d074a3ef05 100644 --- a/doc/development/utilities.md +++ b/doc/development/utilities.md @@ -135,3 +135,44 @@ We developed a number of utilities to ease development. Find.new.clear_memoization(:result) ``` + +## [`RequestCache`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/cache/request_cache.rb) + +This module provides a simple way to cache values in RequestStore, +and the cache key would be based on the class name, method name, +optionally customized instance level values, optionally customized +method level values, and optional method arguments. + +A simple example that only uses the instance level customised values: + +``` ruby +class UserAccess + extend Gitlab::Cache::RequestCache + + request_cache_key do + [user&.id, project&.id] + end + + request_cache def can_push_to_branch?(ref) + # ... + end +end +``` + +This way, the result of `can_push_to_branch?` would be cached in +`RequestStore.store` based on the cache key. If `RequestStore` is not +currently active, then it would be stored in a hash saved in an +instance variable, so the cache logic would be the same. + +We can also set different strategies for different methods: + +``` ruby +class Commit + extend Gitlab::Cache::RequestCache + + def author + User.find_by_any_email(author_email.downcase) + end + request_cache(:author) { author_email.downcase } +end +``` diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md index f502866333e..47396666879 100644 --- a/doc/development/what_requires_downtime.md +++ b/doc/development/what_requires_downtime.md @@ -195,22 +195,22 @@ end And that's it, we're done! -## Changing Column Types For Large Tables +## Changing The Schema For Large Tables -While `change_column_type_concurrently` can be used for changing the type of a -column without downtime it doesn't work very well for large tables. Because all -of the work happens in sequence the migration can take a very long time to -complete, preventing a deployment from proceeding. -`change_column_type_concurrently` can also produce a lot of pressure on the -database due to it rapidly updating many rows in sequence. +While `change_column_type_concurrently` and `rename_column_concurrently` can be +used for changing the schema of a table without downtime it doesn't work very +well for large tables. Because all of the work happens in sequence the migration +can take a very long time to complete, preventing a deployment from proceeding. +They can also produce a lot of pressure on the database due to it rapidly +updating many rows in sequence. To reduce database pressure you should instead use -`change_column_type_using_background_migration` when migrating a column in a -large table (e.g. `issues`). This method works similar to -`change_column_type_concurrently` but uses background migration to spread the -work / load over a longer time period, without slowing down deployments. +`change_column_type_using_background_migration` or `rename_column_concurrently` +when migrating a column in a large table (e.g. `issues`). These methods work +similarly to the concurrent counterparts but uses background migration to spread +the work / load over a longer time period, without slowing down deployments. -Usage of this method is fairly simple: +For example, to change the column type using a background migration: ```ruby class ExampleMigration < ActiveRecord::Migration @@ -296,6 +296,15 @@ class MigrateRemainingIssuesClosedAt < ActiveRecord::Migration end ``` +The same applies to `rename_column_using_background_migration`: + +1. Create a migration using the helper, which will schedule background + migrations to spread the writes over a longer period of time. +2. In the next monthly release, create a clean-up migration to steal from the + Sidekiq queues, migrate any missing rows, and cleanup the rename. This + migration should skip the steps after stealing from the Sidekiq queues if the + column has already been renamed. + For more information, see [the documentation on cleaning up background migrations](background_migrations.md#cleaning-up). |