diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-22 11:31:16 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-22 11:31:16 +0000 |
| commit | 905c1110b08f93a19661cf42a276c7ea90d0a0ff (patch) | |
| tree | 756d138db422392c00471ab06acdff92c5a9b69c /doc/development | |
| parent | 50d93f8d1686950fc58dda4823c4835fd0d8c14b (diff) | |
| download | gitlab-ce-905c1110b08f93a19661cf42a276c7ea90d0a0ff.tar.gz | |
Add latest changes from gitlab-org/gitlab@12-4-stable-ee
Diffstat (limited to 'doc/development')
100 files changed, 1984 insertions, 1137 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 0d1168c4450..16b073045cc 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -19,7 +19,6 @@ description: 'Learn how to contribute to GitLab.' - [Code review guidelines](code_review.md) for reviewing code and having code reviewed - [Database review guidelines](database_review.md) for reviewing database-related changes and complex SQL queries - [Pipelines for the GitLab project](pipelines.md) -- [Automatic CE->EE merge](automatic_ce_ee_merge.md) - [Guidelines for implementing Enterprise Edition features](ee_features.md) - [Security process for developers](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#security-releases-critical-non-critical-as-a-developer) - [Requesting access to Chatops on GitLab.com](chatops_on_gitlabcom.md#requesting-access) (for GitLabbers) @@ -34,6 +33,7 @@ description: 'Learn how to contribute to GitLab.' ## Backend guides - [GitLab utilities](utilities.md) +- [Issuable-like Rails models](issuable-like-models.md) - [Logging](logging.md) - [API styleguide](api_styleguide.md) Use this styleguide if you are contributing to the API @@ -68,6 +68,7 @@ description: 'Learn how to contribute to GitLab.' - [Git LFS](lfs.md) - [Developing against interacting components or features](interacting_components.md) - [File uploads](uploads.md) +- [Auto DevOps development guide](auto_devops.md) ## Performance guides @@ -94,9 +95,11 @@ description: 'Learn how to contribute to GitLab.' - [What requires downtime?](what_requires_downtime.md) - [SQL guidelines](sql.md) for working with SQL queries - [Migrations style guide](migration_style_guide.md) for creating safe SQL migrations +- [Testing Rails migrations](testing_guide/testing_migrations_guide.md) guide - [Post deployment migrations](post_deployment_migrations.md) - [Background migrations](background_migrations.md) - [Swapping tables](swapping_tables.md) +- [Deleting exiting migrations](deleting_migrations.md) ### Best practices @@ -116,7 +119,7 @@ description: 'Learn how to contribute to GitLab.' - [Database helper modules](database_helpers.md) - [Code comments](code_comments.md) -## Case studies +### Case studies - [Database case study: Filtering by label](filtering_by_label.md) - [Database case study: Namespaces storage statistics](namespaces_storage_statistics.md) @@ -148,6 +151,10 @@ description: 'Learn how to contribute to GitLab.' - [Frontend tracking guide](event_tracking/frontend.md) - [Backend tracking guide](event_tracking/backend.md) +## Experiment Guide + +- [Introduction](experiment_guide/index.md) + ## Build guides - [Building a package for testing purposes](build_test_package.md) @@ -164,6 +171,10 @@ description: 'Learn how to contribute to GitLab.' - [Shell scripting standards and style guidelines](shell_scripting_guide/index.md) +## Other Development guides + +- [Defining relations between files using projections](projections.md) + ## Other GitLab Development Kit (GDK) guides - [Run full Auto DevOps cycle in a GDK instance](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/auto_devops.md) diff --git a/doc/development/adding_database_indexes.md b/doc/development/adding_database_indexes.md index c47ce0f1182..6932858c699 100644 --- a/doc/development/adding_database_indexes.md +++ b/doc/development/adding_database_indexes.md @@ -108,7 +108,7 @@ This query outputs a list containing all indexes that are never used and sorts them by indexes sizes in descending order. This query can be useful to determine if any previously indexes are useful after all. More information on the meaning of the various columns can be found at -<https://www.postgresql.org/docs/current/static/monitoring-stats.html>. +<https://www.postgresql.org/docs/current/monitoring-stats.html>. Because the output of this query relies on the actual usage of your database it may be affected by factors such as (but not limited to): diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index c3165dc2e21..cdd0e9b2a7b 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -526,7 +526,7 @@ Using these helpers, we can build specs like this: let(:mutation) do graphql_mutation( :merge_request_set_wip, - project_path: 'gitlab-org/gitlab-ce', + project_path: 'gitlab-org/gitlab-foss', iid: '1', wip: true ) @@ -539,3 +539,8 @@ it 'returns a successful response' do expect(graphql_mutation_response(:merge_request_set_wip)['errors']).to be_empty end ``` + +## Documentation + +For information on generating GraphQL documentation, see +[Rake tasks related to GraphQL](rake_tasks.md#update-graphql-documentation). diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index e3c8c860062..71963ee0c0a 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -14,7 +14,7 @@ Always use an [Entity] to present the endpoint's payload. ## Methods and parameters description Every method must be described using the [Grape DSL](https://github.com/ruby-grape/grape#describing-methods) -(see <https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/api/environments.rb> +(see <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/environments.rb> for a good example): - `desc` for the method summary. You should pass it a block for additional @@ -104,6 +104,11 @@ For instance: - endpoint = expose_path(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: @issue.iid)) ``` -[Entity]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/api/entities.rb +## Internal API + +The [internal API](./internal_api.md) is documented for internal use. Please keep it up to date so we know what endpoints +different components are making use of. + +[Entity]: https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/entities.rb [validation, and coercion of the parameters]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion [installing GitLab under a relative URL]: https://docs.gitlab.com/ee/install/relative_url.html diff --git a/doc/development/architecture.md b/doc/development/architecture.md index 75562f692ad..ccedb96d27d 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -1,7 +1,3 @@ ---- -table_display_block: true ---- - # GitLab Architecture Overview ## Software delivery @@ -16,7 +12,7 @@ Both EE and CE require some add-on components called GitLab Shell and Gitaly. Th ## Components -A typical install of GitLab will be on GNU/Linux. It uses Nginx or Apache as a web front end to proxypass the Unicorn web server. By default, communication between Unicorn and the front end is via a Unix domain socket but forwarding requests via TCP is also supported. The web front end accesses `/home/git/gitlab/public` bypassing the Unicorn server to serve static pages, uploads (e.g. avatar images or attachments), and precompiled assets. GitLab serves web pages and a [GitLab API](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/doc/api) using the Unicorn web server. It uses Sidekiq as a job queue which, in turn, uses redis as a non-persistent database backend for job information, meta data, and incoming jobs. +A typical install of GitLab will be on GNU/Linux. It uses NGINX or Apache as a web front end to proxypass the Unicorn web server. By default, communication between Unicorn and the front end is via a Unix domain socket but forwarding requests via TCP is also supported. The web front end accesses `/home/git/gitlab/public` bypassing the Unicorn server to serve static pages, uploads (e.g. avatar images or attachments), and precompiled assets. GitLab serves web pages and the [GitLab API](../api/README.md) using the Unicorn web server. It uses Sidekiq as a job queue which, in turn, uses Redis as a non-persistent database backend for job information, meta data, and incoming jobs. We also support deploying GitLab on Kubernetes using our [GitLab Helm chart](https://docs.gitlab.com/charts/). @@ -24,7 +20,7 @@ The GitLab web app uses PostgreSQL for persistent database information (e.g. use When serving repositories over HTTP/HTTPS GitLab utilizes the GitLab API to resolve authorization and access as well as serving Git objects. -The add-on component GitLab Shell serves repositories over SSH. It manages the SSH keys within `/home/git/.ssh/authorized_keys` which should not be manually edited. GitLab Shell accesses the bare repositories through Gitaly to serve Git objects and communicates with redis to submit jobs to Sidekiq for GitLab to process. GitLab Shell queries the GitLab API to determine authorization and access. +The add-on component GitLab Shell serves repositories over SSH. It manages the SSH keys within `/home/git/.ssh/authorized_keys` which should not be manually edited. GitLab Shell accesses the bare repositories through Gitaly to serve Git objects and communicates with Redis to submit jobs to Sidekiq for GitLab to process. GitLab Shell queries the GitLab API to determine authorization and access. Gitaly executes Git operations from GitLab Shell and the GitLab web app, and provides an API to the GitLab web app to get attributes from Git (e.g. title, branches, tags, other meta data), and to get blobs (e.g. diffs, commits, files). @@ -161,7 +157,7 @@ Component statuses are linked to configuration documentation for each component. | [Elasticsearch](#elasticsearch) | Improved search within GitLab | [⤓][elasticsearch-omnibus] | [⤓][elasticsearch-charts] | [⤓][elasticsearch-charts] | [❌](https://gitlab.com/groups/gitlab-org/-/epics/153) | [⤓][elasticsearch-source] | [⤓][elasticsearch-gdk] | EE Only | | [Sentry integration](#sentry) | Error tracking for deployed apps | [⤓][sentry-integration] | [⤓][sentry-integration] | [⤓][sentry-integration] | [⤓][sentry-integration] | [⤓][sentry-integration] | [⤓][sentry-integration] | CE & EE | | [Jaeger integration](#jaeger) | Distributed tracing for deployed apps | [⤓][jaeger-integration] | [⤓][jaeger-integration] | [⤓][jaeger-integration] | [⤓][jaeger-integration] | [⤓][jaeger-integration] | [⤓][jaeger-integration] | EE Only | -| [GitLab Managed Apps](#gitlab-managed-apps) | Deploy [Helm](https://docs.helm.sh/), [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/), [Cert-Manager](https://docs.cert-manager.io/en/latest/), [Prometheus](https://prometheus.io/docs/introduction/overview/), a [Runner](https://docs.gitlab.com/runner/), [JupyterHub](http://jupyter.org/), [Knative](https://cloud.google.com/knative) to a cluster | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | CE & EE | +| [GitLab Managed Apps](#gitlab-managed-apps) | Deploy [Helm](https://helm.sh/docs/), [Ingress](https://kubernetes.io/docs/concepts/services-networking/ingress/), [Cert-Manager](https://docs.cert-manager.io/en/latest/), [Prometheus](https://prometheus.io/docs/introduction/overview/), a [Runner](https://docs.gitlab.com/runner/), [JupyterHub](https://jupyter.org), [Knative](https://cloud.google.com/knative/) to a cluster | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | [⤓][managed-k8s-apps] | CE & EE | ### Component details @@ -228,7 +224,7 @@ Gitaly is a service designed by GitLab to remove our need for NFS for Git storag - Configuration: [Omnibus][geo-omnibus], [Charts][geo-charts], [GDK][geo-gdk] - Layer: Core Service (Processor) -#### Gitlab Exporter +#### GitLab Exporter - [Project page](https://gitlab.com/gitlab-org/gitlab-exporter) - Configuration: [Omnibus][gitlab-exporter-omnibus], [Charts][gitlab-exporter-charts] @@ -262,7 +258,7 @@ GitLab CI is the open-source continuous integration service included with GitLab - Configuration: [Omnibus][shell-omnibus], [Charts][shell-charts], [Source][shell-source], [GDK][gitlab-yml] - Layer: Core Service (Processor) -[GitLab Shell](https://gitlab.com/gitlab-org/gitlab-shell) is a program designed at GitLab to handle ssh-based `git` sessions, and modifies the list of authorized keys. GitLab Shell is not a Unix shell nor a replacement for Bash or Zsh. +[GitLab Shell](https://gitlab.com/gitlab-org/gitlab-shell) is a program designed at GitLab to handle SSH-based `git` sessions, and modifies the list of authorized keys. GitLab Shell is not a Unix shell nor a replacement for Bash or Zsh. #### GitLab Workhorse @@ -271,7 +267,7 @@ GitLab CI is the open-source continuous integration service included with GitLab - Layer: Core Service (Processor) - Process: `gitlab-workhorse` -[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) is a program designed at GitLab to help alleviate pressure from Unicorn. You can read more about the [historical reasons for developing](https://about.gitlab.com/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. +[GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse) is a program designed at GitLab to help alleviate pressure from Unicorn. You can read more about the [historical reasons for developing](https://about.gitlab.com/blog/2016/04/12/a-brief-history-of-gitlab-workhorse/). It's designed to act as a smart reverse proxy to help speed up GitLab as a whole. #### Grafana @@ -321,7 +317,7 @@ MinIO is an object storage server released under Apache License v2.0. It is comp - Layer: Core Service (Processor) - Process: `nginx` -Nginx as an ingress port for all HTTP requests and routes them to the appropriate sub-systems within GitLab. We are bundling an unmodified version of the popular open source webserver. +NGINX has an Ingress port for all HTTP requests and routes them to the appropriate sub-systems within GitLab. We are bundling an unmodified version of the popular open source webserver. #### Node Exporter @@ -348,7 +344,7 @@ Lightweight connection pooler for PostgreSQL. Prometheus exporter for PgBouncer. Exports metrics at 9127/metrics. -#### Postgresql +#### PostgreSQL - [Project page](https://github.com/postgres/postgres/blob/master/README) - Configuration: [Omnibus][postgres-omnibus], [Charts][postgres-charts], [Source][postgres-source] @@ -404,7 +400,7 @@ Redis is packaged to provide a place to store: - Layer: Core Service (Processor) The registry is what users use to store their own Docker images. The bundled -registry uses nginx as a load balancer and GitLab as an authentication manager. +registry uses NGINX as a load balancer and GitLab as an authentication manager. Whenever a client requests to pull or push an image from the registry, it will return a `401` response along with a header detailing where to get an authentication token, in this case the GitLab instance. The client will then @@ -428,7 +424,7 @@ Sentry fundamentally is a service that helps you monitor and fix crashes in real - Layer: Core Service (Processor) - Process: `sidekiq` -Sidekiq is a Ruby background job processor that pulls jobs from the redis queue and processes them. Background jobs allow GitLab to provide a faster request/response cycle by moving work into the background. +Sidekiq is a Ruby background job processor that pulls jobs from the Redis queue and processes them. Background jobs allow GitLab to provide a faster request/response cycle by moving work into the background. #### Unicorn @@ -474,9 +470,9 @@ It's important to understand the distinction as some processes are used in both When making a request to an HTTP Endpoint (think `/users/sign_in`) the request will take the following path through the GitLab Service: -- nginx - Acts as our first line reverse proxy. +- NGINX - Acts as our first line reverse proxy. - GitLab Workhorse - This determines if it needs to go to the Rails application or somewhere else to reduce load on Unicorn. -- unicorn - Since this is a web request, and it needs to access the application it will go to Unicorn. +- Unicorn - Since this is a web request, and it needs to access the application it will go to Unicorn. - Postgres/Gitaly/Redis - Depending on the type of request, it may hit these services to store or retrieve data. ### GitLab Git Request Cycle @@ -512,7 +508,7 @@ ps aux | grep '^git' ``` GitLab has several components to operate. It requires a persistent database -(PostgreSQL) and redis database, and uses Apache httpd or Nginx to proxypass +(PostgreSQL) and Redis database, and uses Apache httpd or NGINX to proxypass Unicorn. All these components should run as different system users to GitLab (e.g., `postgres`, `redis` and `www-data`, instead of `git`). @@ -584,7 +580,7 @@ SSH: - `/var/log/auth.log` auth log (on Ubuntu). - `/var/log/secure` auth log (on RHEL). -nginx: +NGINX: - `/var/log/nginx/` contains error and access logs. @@ -614,7 +610,7 @@ GitLab Shell has a configuration file at `/home/git/gitlab-shell/config.yml`. ### Maintenance Tasks -[GitLab](https://gitlab.com/gitlab-org/gitlab/tree/master) provides rake tasks with which you see version information and run a quick check on your configuration to ensure it is configured properly within the application. See [maintenance rake tasks](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/doc/raketasks/maintenance.md). +[GitLab](https://gitlab.com/gitlab-org/gitlab/tree/master) provides rake tasks with which you see version information and run a quick check on your configuration to ensure it is configured properly within the application. See [maintenance rake tasks](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/raketasks/maintenance.md). In a nutshell, do the following: ``` @@ -638,7 +634,7 @@ We've also detailed [our architecture of GitLab.com](https://about.gitlab.com/ha [unicorn-omnibus]: https://docs.gitlab.com/omnibus/settings/unicorn.html [unicorn-charts]: https://docs.gitlab.com/charts/charts/gitlab/unicorn/ [unicorn-source]: ../install/installation.md#configure-it -[gitlab-yml]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/config/gitlab.yml.example +[gitlab-yml]: https://gitlab.com/gitlab-org/gitlab/blob/master/config/gitlab.yml.example [sidekiq-omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/files/gitlab-config-template/gitlab.rb.template [sidekiq-charts]: https://docs.gitlab.com/charts/charts/gitlab/sidekiq/ [gitaly-omnibus]: ../administration/gitaly/index.md diff --git a/doc/development/auto_devops.md b/doc/development/auto_devops.md new file mode 100644 index 00000000000..f88bcc9cfb8 --- /dev/null +++ b/doc/development/auto_devops.md @@ -0,0 +1,42 @@ +# Auto DevOps development guide + +This document provides a development guide for contributors to +[Auto DevOps](../topics/autodevops/index.md) + +## Development + +Auto DevOps builds on top of GitLab CI to create an automatic pipeline +based on your project contents. When Auto DevOps is enabled for a +project, the user does not need to explicitly include any pipeline configuration +through a [`.gitlab-ci.yml` file](../ci/yaml/README.md). + +In the absence of a `.gitlab-ci.yml` file, the [Auto DevOps CI +template](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml) +is used implicitly to configure the pipeline for the project. This +template is a top-level template that includes other sub-templates, +which then defines jobs. + +Some jobs use images that are built from external projects: + +- [Auto Build](../topics/autodevops/index.md#auto-build) uses + [configuration](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml) + in which the `build` job uses an image that is built using the + [`auto-build-image`](https://gitlab.com/gitlab-org/cluster-integration/auto-build-image) + project. +- [Auto Deploy](../topics/autodevops/index.md#auto-deploy) uses + [configuration](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml) + in which the jobs defined in this template use an image that is built using the + [`auto-deploy-image`](https://gitlab.com/gitlab-org/cluster-integration/auto-deploy-image) + project. By default, the Helm chart defined in + [`auto-deploy-app`](https://gitlab.com/gitlab-org/charts/auto-deploy-app) + is used to deploy. + +There are extra variables that get passed to the CI jobs when Auto +DevOps is enabled that are not present in a normal CI job. These can be +found in +[`ProjectAutoDevops`](https://gitlab.com/gitlab-org/gitlab/blob/bf69484afa94e091c3e1383945f60dbe4e8681af/app/models/project_auto_devops.rb). + +## Development environment + +Configuring [GDK for Auto +DevOps](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/auto_devops.md). diff --git a/doc/development/automatic_ce_ee_merge.md b/doc/development/automatic_ce_ee_merge.md deleted file mode 100644 index f7816091b49..00000000000 --- a/doc/development/automatic_ce_ee_merge.md +++ /dev/null @@ -1,243 +0,0 @@ -# Automatic CE->EE merge - -Commits pushed to CE `master` are automatically merged into EE `master` roughly -every 5 minutes. Changes are merged using the `recursive=ours` merge strategy in -the context of EE. This means that any merge conflicts are resolved by taking -the EE changes and discarding the CE changes. This removes the need for -resolving conflicts or reverting changes, at the cost of **absolutely -requiring** EE merge requests to be created whenever a CE merge request causes -merge conflicts. Failing to do so can result in changes not making their way -into EE. - -## Always create an EE merge request if there are conflicts - -In CI there is a job called `ee_compat_check`, which checks if a CE MR causes -merge conflicts with EE. If this job reports conflicts, you **must** create an -EE merge request. If you are an external contributor you can ask the reviewer to -do this for you. - -## Always merge EE merge requests before their CE counterparts - -**In order to avoid conflicts in the CE->EE merge, you should always merge the -EE version of your CE merge request first, if present.** - -Failing to do so will lead to CE changes being discarded when merging into EE, -if they cause merge conflicts. - -## Avoiding CE->EE merge conflicts beforehand - -To avoid the conflicts beforehand, check out the -[Guidelines for implementing Enterprise Edition features](ee_features.md). - -In any case, the CI `ee_compat_check` job will tell you if you need to open an -EE version of your CE merge request. - -### Conflicts detection in CE merge requests - -For each commit (except on `master`), the `ee_compat_check` CI job tries to -detect if the current branch's changes will conflict during the CE->EE merge. - -The job reports what files are conflicting and how to set up a merge request -against EE. - -#### How the job works - -1. Generates the diff between your branch and current CE `master` -1. Tries to apply it to current EE `master` -1. If it applies cleanly, the job succeeds, otherwise... -1. Detects a branch with the `ee-` prefix or `-ee` suffix in EE -1. If it exists, generate the diff between this branch and current EE `master` -1. Tries to apply it to current EE `master` -1. If it applies cleanly, the job succeeds - -In the case where the job fails, it means you should create an `ee-<ce_branch>` -or `<ce_branch>-ee` branch, push it to EE and open a merge request against EE -`master`. -At this point if you retry the failing job in your CE merge request, it should -now pass. - -Notes: - -- This task is not a silver-bullet, its current goal is to bring awareness to - developers that their work needs to be ported to EE. -- Community contributors shouldn't be required to submit merge requests against - EE, but reviewers should take actions by either creating such EE merge request - or asking a GitLab developer to do it **before the merge request is merged**. -- If you branch is too far behind `master`, the job will fail. In that case you - should rebase your branch upon latest `master`. -- Code reviews for merge requests often consist of multiple iterations of - feedback and fixes. There is no need to update your EE MR after each - iteration. Instead, create an EE MR as soon as you see the - `ee_compat_check` job failing. After you receive the final approval - from a Maintainer (but **before the CE MR is merged**) update the EE MR. - This helps to identify significant conflicts sooner, but also reduces the - number of times you have to resolve conflicts. -- Please remember to - [always have your EE merge request merged before the CE version](#always-merge-ee-merge-requests-before-their-ce-counterparts). -- You can use [`git rerere`](https://git-scm.com/docs/git-rerere) - to avoid resolving the same conflicts multiple times. - -### Cherry-picking from CE to EE - -For avoiding merge conflicts, we use a method of creating equivalent branches -for CE and EE. If the `ee-compat-check` job fails, this process is required. - -This method only requires that you have cloned both CE and EE into your computer. -If you don't have them yet, please go ahead and clone them: - -- Clone CE repo: `git clone git@gitlab.com:gitlab-org/gitlab-ce.git` -- Clone EE repo: `git clone git@gitlab.com:gitlab-org/gitlab-ee.git` - -And the only additional setup we need is to add CE as remote of EE and vice-versa: - -- Open two terminal windows, one in CE, and another one in EE: - - In EE: `git remote add ce git@gitlab.com:gitlab-org/gitlab-ce.git` - - In CE: `git remote add ee git@gitlab.com:gitlab-org/gitlab-ee.git` - -That's all setup we need, so that we can cherry-pick a commit from CE to EE, and -from EE to CE. - -Now, every time you create an MR for CE and EE: - -1. Open two terminal windows, one in CE, and another one in EE -1. In the CE terminal: - 1. Create the CE branch, e.g., `branch-example` - 1. Make your changes and push a commit (commit A) - 1. Create the CE merge request in GitLab -1. In the EE terminal: - 1. Create the EE-equivalent branch ending with `-ee`, e.g., - `git checkout -b branch-example-ee` - 1. Fetch the CE branch: `git fetch ce branch-example` - 1. Cherry-pick the commit A: `git cherry-pick commit-A-SHA` - 1. If Git prompts you to fix the conflicts, do a `git status` - to check which files contain conflicts, fix them, save the files - 1. Add the changes with `git add .` but **DO NOT commit** them - 1. Continue cherry-picking: `git cherry-pick --continue` - 1. Push to EE: `git push origin branch-example-ee` -1. Create the EE-equivalent MR and link to the CE MR from the - description `Ports [CE-MR-LINK] to EE` -1. Once all the jobs are passing in both CE and EE, you've addressed the - feedback from your own team, and got them approved, the merge requests can be merged. -1. When both MRs are ready, the EE merge request will be merged first, and the - CE-equivalent will be merged next. - -**Important notes:** - -- The commit SHA can be easily found from the GitLab UI. From a merge request, - open the tab **Commits** and click the copy icon to copy the commit SHA. -- To cherry-pick a **commit range**, such as (A > B > C > D) use: - - ```shell - git cherry-pick "oldest-commit-SHA^..newest-commit-SHA" - ``` - - For example, suppose the commit A is the oldest, and its SHA is `4f5e4018c09ed797fdf446b3752f82e46f5af502`, - and the commit D is the newest, and its SHA is `80e1c9e56783bd57bd7129828ec20b252ebc0538`. - The cherry-pick command will be: - - ```shell - git cherry-pick "4f5e4018c09ed797fdf446b3752f82e46f5af502^..80e1c9e56783bd57bd7129828ec20b252ebc0538" - ``` - -- To cherry-pick a **merge commit**, use the flag `-m 1`. For example, suppose that the - merge commit SHA is `138f5e2f20289bb376caffa0303adb0cac859ce1`: - - ```shell - git cherry-pick -m 1 138f5e2f20289bb376caffa0303adb0cac859ce1 - ``` - -- To cherry-pick multiple commits, such as B and D in a range (A > B > C > D), use: - - ```shell - git cherry-pick commit-B-SHA commit-D-SHA - ``` - - For example, suppose commit B SHA = `4f5e4018c09ed797fdf446b3752f82e46f5af502`, - and the commit D SHA = `80e1c9e56783bd57bd7129828ec20b252ebc0538`. - The cherry-pick command will be: - - ```shell - git cherry-pick 4f5e4018c09ed797fdf446b3752f82e46f5af502 80e1c9e56783bd57bd7129828ec20b252ebc0538 - ``` - - This case is particularly useful when you have a merge commit in a sequence of - commits and you want to cherry-pick all but the merge commit. - -- If you push more commits to the CE branch, you can safely repeat the procedure - to cherry-pick them to the EE-equivalent branch. You can do that as many times as - necessary, using the same CE and EE branches. -- If you submitted the merge request to the CE repo and the `ee-compat-check` job passed, - you are not required to submit the EE-equivalent MR, but it's still recommended. If the - job failed, you are required to submit the EE MR so that you can fix the conflicts in EE - before merging your changes into CE. - -## How we run the Automatic CE->EE merge at GitLab - -At GitLab, we use the [Merge Train](https://gitlab.com/gitlab-org/merge-train) -project to keep our [GitLab EE](https://gitlab.com/gitlab-org/gitlab) -repository updated with commits from -[GitLab CE](https://gitlab.com/gitlab-org/gitlab-foss). - -We have a mirror of the [Merge Train](https://gitlab.com/gitlab-org/merge-train) -project [configured](https://ops.gitlab.net/gitlab-org/merge-train) to run an -automatic CE->EE merge job every twenty minutes as a scheduled CI job. The -[configured](https://ops.gitlab.net/gitlab-org/merge-train) Merge Train project -is only accessible to authorized GitLab staff. - -## FAQ - -### How does automatic merging work? - -The automatic merging is performed using a project called [Merge -Train](https://gitlab.com/gitlab-org/merge-train/). This project will clone CE -and EE master, and merge CE master into EE master using `git merge ---strategy=recursive --strategy-option=ours`. This process runs multiple times -per hour. - -For more information on the exact implementation you can refer to the source -code. - -### Why merge automatically? - -As we work towards continuous deployments and a single repository for both CE -and EE, we need to first make sure that all CE changes make their way into EE as -fast as possible. Past experiences and data have shown that periodic CE to EE -merge requests do not scale, and often take a very long time to complete. For -example, [in this -comment](https://gitlab.com/gitlab-org/release/framework/issues/49#note_114614619) -we determined that the average time to close an upstream merge request is around -5 hours, with peaks up to several days. Periodic merge requests are also -frustrating to work with, because they often include many changes unrelated to -your own changes. - -To resolve these problems, we now merge changes using the `ours` strategy to -automatically resolve merge conflicts. This removes the need for resolving -conflicts in a periodic merge request, and allows us to merge changes from CE -into EE much faster. - -### My CE merge request caused conflicts after it was merged. What do I do? - -If you notice this, you should set up an EE merge request that resolves these -conflicts as **soon as possible**. Failing to do so can lead to your changes not -being available in EE, which may break tests. This in turn would prevent us from -being able to deploy. - -### Won't this setup be risky? - -No, not if there is an EE merge request for every CE merge request that causes -conflicts _and_ that EE merge request is merged first. In the past we may have -been a bit more relaxed when it comes to enforcing EE merge requests, but to -enable automatic merging we have to start requiring such merge requests even for -the smallest conflicts. - -### Some files I work with often conflict, how can I best deal with this? - -If you find you keep running into merge conflicts, consider refactoring the file -so that the EE specific changes are not intertwined with CE code. For Ruby code -you can do this by moving the EE code to a separate module, which can then be -injected into the appropriate classes or modules. See [Guidelines for -implementing Enterprise Edition features](ee_features.md) for more information. - ---- - -[Return to Development documentation](README.md) diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 364e276b6cc..0a08360b727 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -290,7 +290,9 @@ It is required to write tests for: - A cleanup migration. You can use the `:migration` RSpec tag when testing the migrations. -See [README][migrations-readme]. +See the +[Testing Rails migrations](testing_guide/testing_migrations_guide.md#testing-a-non-activerecordmigration-class) +style guide. When you do that, keep in mind that `before` and `after` RSpec hooks are going to migrate you database down and up, which can result in other background diff --git a/doc/development/build_test_package.md b/doc/development/build_test_package.md index 21891f70d73..f58ac79b6f4 100644 --- a/doc/development/build_test_package.md +++ b/doc/development/build_test_package.md @@ -9,7 +9,7 @@ that will create: - A deb package for Ubuntu 16.04, available as a build artifact, and - A docker image, which is pushed to [Omnibus GitLab's container registry](https://gitlab.com/gitlab-org/omnibus-gitlab/container_registry) - (images titled `gitlab-ce` and `gitlab-ee` respectively and image tag is the + (images titled `gitlab-foss` and `gitlab-ee` respectively and image tag is the commit which triggered the pipeline). When you push a commit to either the GitLab CE or GitLab EE project, the diff --git a/doc/development/changelog.md b/doc/development/changelog.md index cd09438e7c7..8ded3f393ee 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -33,8 +33,13 @@ the `author` field. GitLab team members **should not**. ## What warrants a changelog entry? +- Any change that introduces a database migration **must** have a changelog entry. - Any user-facing change **should** have a changelog entry. Example: "GitLab now uses system fonts for all text." +- Performance improvements **should** have a changelog entry. +- _Any_ contribution from a community member, no matter how small, **may** have + a changelog entry regardless of these guidelines if the contributor wants one. + Example: "Fixed a typo on the search results page." - Any docs-only changes **should not** have a changelog entry. - Any change behind a feature flag **should not** have a changelog entry. The entry should be added [in the merge request removing the feature flags](feature_flags/development.md). - A fix for a regression introduced and then fixed in the same release (i.e., @@ -43,12 +48,6 @@ the `author` field. GitLab team members **should not**. - Any developer-facing change (e.g., refactoring, technical debt remediation, test suite changes) **should not** have a changelog entry. Example: "Reduce database records created during Cycle Analytics model spec." -- _Any_ contribution from a community member, no matter how small, **may** have - a changelog entry regardless of these guidelines if the contributor wants one. - Example: "Fixed a typo on the search results page." -- Performance improvements **should** have a changelog entry. -- Any change that introduces a database migration **must** have a - changelog entry. ## Writing good changelog entries @@ -80,7 +79,7 @@ changes. - **Bad:** Strip out `nil`s in the Array of Commit objects returned from `find_commits_by_message_with_elastic` -- **Good:** Fix 500 errors caused by elasticsearch results referencing +- **Good:** Fix 500 errors caused by Elasticsearch results referencing garbage-collected commits The first example focuses on _how_ we fixed something, not on _what_ it fixes. diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 09d6638924a..8a313a120f1 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -13,8 +13,8 @@ tasks such as: To request access to Chatops on GitLab.com: -1. Log into <https://ops.gitlab.net/users/sign_in> using the same username as for GitLab.com. -1. Ask [anyone in the `chatops` project](https://gitlab.com/gitlab-com/chatops/-/project_members) to add you by running `/chatops run member add <username> gitlab-com/chatops --ops`. +1. Log into <https://ops.gitlab.net/users/sign_in> **using the same username** as for GitLab.com (you may have to rename it). +1. Ask [an owner/maintainer in the `chatops` project](https://gitlab.com/gitlab-com/chatops/-/project_members?search=&sort=access_level_desc) to add you by running `/chatops run member add <username> gitlab-com/chatops --ops`. ## See also diff --git a/doc/development/code_review.md b/doc/development/code_review.md index f616eb90bda..421b70bd2db 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -18,7 +18,7 @@ recommended to pick someone who knows the domain well. You can read more about t importance of involving reviewer(s) in the section on the responsibility of the author below. If you need some guidance (e.g. it's your first merge request), feel free to ask -one of the [Merge request coaches][team]. +one of the [Merge request coaches](https://about.gitlab.com/company/team/). If you need assistance with security scans or comments, feel free to include the Security Team (`@gitlab-com/gl-security`) in the review. @@ -66,13 +66,13 @@ from teams other than your own. 1. If your merge request includes frontend changes [^1], it must be **approved by a [frontend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_frontend)**. 1. If your merge request includes UX changes [^1], it must be - **approved by a [UX team member][team]**. + **approved by a [UX team member](https://about.gitlab.com/company/team/)**. 1. If your merge request includes adding a new JavaScript library [^1], it must be - **approved by a [frontend lead][team]**. + **approved by a [frontend lead](https://about.gitlab.com/company/team/)**. 1. If your merge request includes adding a new UI/UX paradigm [^1], it must be - **approved by a [UX lead][team]**. + **approved by a [UX lead](https://about.gitlab.com/company/team/)**. 1. If your merge request includes a new dependency or a filesystem change, it must be - **approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/#how-to-work-with-distribution) for more details. + **approved by a [Distribution team member](https://about.gitlab.com/company/team/)**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/#how-to-work-with-distribution) for more details. #### Security requirements @@ -172,7 +172,7 @@ required approvers. Maintainers must check before merging if the merge request is introducing new vulnerabilities, by inspecting the list in the Merge Request [Security Widget](../user/project/merge_requests/index.md#security-reports-ultimate). -When in doubt, a [Security Engineer][team] can be involved. The list of detected +When in doubt, a [Security Engineer](https://about.gitlab.com/company/team/) can be involved. The list of detected vulnerabilities must be either empty or containing: - dismissed vulnerabilities in case of false positives @@ -256,18 +256,12 @@ Since [unblocking others is always a top priority](https://about.gitlab.com/hand reviewers are expected to review assigned merge requests in a timely manner, even when this may negatively impact their other tasks and priorities. Doing so allows everyone involved in the merge request to iterate faster as the -context is fresh in memory, improves contributors' experiences significantly, -and gives authors more time to address feedback and iterate on their work before -the [feature freeze](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md#feature-freeze-on-the-7th-for-the-release-on-the-22nd). +context is fresh in memory, improves contributors' experiences significantly. A turnaround time of two working days is usually acceptable, since engineers will typically have other things to work on while they're waiting for review, but don't hesitate to ask the author if it's unclear what time frame would be -acceptable, how urgent the review is, or how significant the blockage. Authors -are also encouraged to provide this information up-front to reviewers, but are -expected to be mindful of the [guidelines on when to ask for review on MRs that -are intended to go in before the feature freeze](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md#between-the-1st-and-the-7th), -and realistic in their expectations if these were not followed. +acceptable, how urgent the review is, or how significant the blockage. If you don't think you'll be able to review a merge request within a reasonable time frame, let the author know as soon as possible and try to help them find @@ -433,7 +427,6 @@ Largely based on the [thoughtbot code review guide]. [Return to Development documentation](README.md) [projects]: https://about.gitlab.com/handbook/engineering/projects/ -[team]: https://about.gitlab.com/team/ [build handbook]: https://about.gitlab.com/handbook/build/handbook/build#how-to-work-with-build [^1]: Please note that specs other than JavaScript specs are considered backend code. [^2]: We encourage you to seek guidance from a database maintainer if your merge request is potentially introducing expensive queries. It is most efficient to comment on the line of code in question with the SQL queries so they can give their advice. diff --git a/doc/development/contributing/design.md b/doc/development/contributing/design.md index 79750878aac..9796e195f86 100644 --- a/doc/development/contributing/design.md +++ b/doc/development/contributing/design.md @@ -5,7 +5,7 @@ For guidance on UX implementation at GitLab, please refer to our [Design System] The UX team uses labels to manage their workflow. The ~"UX" label on an issue is a signal to the UX team that it will need UX attention. -To better understand the priority by which UX tackles issues, see the [UX section](https://about.gitlab.com/handbook/engineering/ux) of the handbook. +To better understand the priority by which UX tackles issues, see the [UX section](https://about.gitlab.com/handbook/engineering/ux/) of the handbook. Once an issue has been worked on and is ready for development, a UXer removes the ~"UX" label and applies the ~"UX ready" label to that issue. diff --git a/doc/development/contributing/index.md b/doc/development/contributing/index.md index 694f8d2cb45..92dd040a2bd 100644 --- a/doc/development/contributing/index.md +++ b/doc/development/contributing/index.md @@ -15,7 +15,7 @@ abbreviation. To get an overview of GitLab community membership including those that would be reviewing or merging your contributions, please visit [the community roles page](community_roles.md). -If you want to know how the GitLab [core team] +If you want to know how the GitLab [core team](https://about.gitlab.com/community/core-team/) operates please see [the GitLab contributing process](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md). [GitLab Inc engineers should refer to the engineering workflow document](https://about.gitlab.com/handbook/engineering/workflow/) @@ -24,7 +24,7 @@ operates please see [the GitLab contributing process](https://gitlab.com/gitlab- Please report suspected security vulnerabilities in private to `support@gitlab.com`, also see the -[disclosure section on the GitLab.com website](https://about.gitlab.com/disclosure/). +[disclosure section on the GitLab.com website](https://about.gitlab.com/security/disclosure/). Please do **NOT** create publicly viewable issues for suspected security vulnerabilities. @@ -48,7 +48,7 @@ for audiences of all ages. If a contributor is no longer actively working on a submitted merge request we can decide that the merge request will be finished by one of our -[Merge request coaches][team] or close the merge request. We make this decision +[Merge request coaches](https://about.gitlab.com/company/team/) or close the merge request. We make this decision based on how important the change is for our product vision. If a merge request coach is going to finish the merge request we assign the ~"coach will finish" label. When a team member picks up a community contribution, @@ -59,18 +59,17 @@ within the MR. ## Helping others Please help other GitLab users when you can. -The methods people will use to seek help can be found on the [getting help page][getting-help]. +The methods people will use to seek help can be found on the [getting help page](https://about.gitlab.com/get-help/). Sign up for the mailing list, answer GitLab questions on StackOverflow or -respond in the IRC channel. You can also sign up on [CodeTriage][codetriage] to help with -the remaining issues on the GitHub issue tracker. +respond in the IRC channel. ## I want to contribute If you want to contribute to GitLab, [issues with the `Accepting merge requests` label](issue_workflow.md#label-for-community-contributors) are a great place to start. -If you have any questions or need help visit [Getting Help](https://about.gitlab.com/getting-help/#discussion) to +If you have any questions or need help visit [Getting Help](https://about.gitlab.com/get-help/) to learn how to communicate with GitLab. If you're looking for a Gitter or Slack channel please consider we favor [asynchronous communication](https://about.gitlab.com/handbook/communication/#internal-communication) over real time communication. Thanks for your contribution! @@ -85,7 +84,7 @@ When maintainers are reading through a merge request they may request guidance f Sometimes style guides will be followed but the code will lack structural integrity, or the maintainer will have reservations about the code’s overall quality. When there is a reservation the maintainer will inform the author and provide some guidance. The author may then choose to update the merge request. Once the merge request has been updated and reassigned to the maintainer, they will review the code again. Once the code has been resubmitted any number of times, the maintainer may choose to close the merge request with a summary of why it will not be merged, as well as some guidance. If the merge request is closed the maintainer will be open to discussion as to how to improve the code so it can be approved in the future. -GitLab will do its best to review community contributions as quickly as possible. Specially appointed developers review community contributions daily. You may take a look at the [team page](https://about.gitlab.com/team/) for the merge request coach who specializes in the type of code you have written and mention them in the merge request. For example, if you have written some JavaScript in your code then you should mention the frontend merge request coach. If your code has multiple disciplines you may mention multiple merge request coaches. +GitLab will do its best to review community contributions as quickly as possible. Specially appointed developers review community contributions daily. You may take a look at the [team page](https://about.gitlab.com/company/team/) for the merge request coach who specializes in the type of code you have written and mention them in the merge request. For example, if you have written some JavaScript in your code then you should mention the frontend merge request coach. If your code has multiple disciplines you may mention multiple merge request coaches. GitLab receives a lot of community contributions, so if your code has not been reviewed within two days (excluding weekend and public holidays) of its initial submission feel free to re-mention the appropriate merge request coach. @@ -122,8 +121,3 @@ This [documentation](style_guides.md) outlines the current style guidelines. --- [Return to Development documentation](../README.md) - -[core team]: https://about.gitlab.com/core-team/ -[team]: https://about.gitlab.com/company/team/ -[getting-help]: https://about.gitlab.com/getting-help/ -[codetriage]: http://www.codetriage.com/gitlabhq/gitlabhq diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index 810d03e82c5..349bb371835 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -22,14 +22,15 @@ once every quarter. The most important thing is making sure valid issues receive feedback from the development team. Therefore the priority is mentioning developers that can help on those issues. Please select someone with relevant experience from the -[GitLab team](https://about.gitlab.com/team/). +[GitLab team](https://about.gitlab.com/company/team/). If there is nobody mentioned with that expertise look in the commit history for the affected files to find someone. -We also use [GitLab Triage](https://gitlab.com/gitlab-org/gitlab-triage) to -automate some triaging policies. This is currently set up as a -[scheduled pipeline](https://gitlab.com/gitlab-org/quality/triage-ops/pipeline_schedules/10512/edit) -running on [quality/triage-ops](https://gitlab.com/gitlab-org/quality/triage-ops) project. +We also use [GitLab Triage](https://gitlab.com/gitlab-org/gitlab-triage) to automate +some triaging policies. This is currently set up as a scheduled pipeline +(`https://gitlab.com/gitlab-org/quality/triage-ops/pipeline_schedules/10512/editpipeline_schedules/10512/edit`, +must have at least developer access to the project) running on [quality/triage-ops](https://gitlab.com/gitlab-org/quality/triage-ops) +project. ## Labels @@ -45,8 +46,8 @@ Most issues will have labels for at least one of the following: - Category: ~"Category:Code Analytics", ~"Category:DevOps Score", ~"Category:Templates", etc. - Feature: ~wiki, ~ldap, ~api, ~issues, ~"merge requests", etc. - Department: ~UX, ~Quality -- Team: ~Documentation, ~Delivery -- Specialization: ~frontend, ~backend +- Team: ~"Technical Writing", ~Delivery +- Specialization: ~frontend, ~backend, ~documentation - Release Scoping: ~Deliverable, ~Stretch, ~"Next Patch Release" - Priority: ~P1, ~P2, ~P3, ~P4 - Severity: ~S1, ~S2, ~S3, ~S4 @@ -96,6 +97,7 @@ Following is a non-exhaustive list of facet labels: - ~security: A security issue could describe a ~bug or a ~feature. - ~database: A database issue could describe a ~bug or a ~feature. - ~customer: This relates to an issue that was created by a customer, or that is of interest for a customer. +- ~"UI text": Issues that add or modify any text within the UI such as user-assistance microcopy, button/menu labels, or error messages. ### Stage labels @@ -148,10 +150,15 @@ You can find the groups listed in the [Product Stages, Groups, and Categories](h We use the term group to map down product requirements from our product stages. As a team needs some way to collect the work their members are planning to be assigned to, we use the `~group::` labels to do so. -Normally there is a 1:1 relationship between Stage labels and Group labels. In the spirit of "Everyone can contribute", -any issue can be picked up by any group, depending on current priorities. For example, an issue labeled ~"devops::create" may be picked up by the ~"group::access" group. +Normally there is a 1:1 relationship between Stage labels and Group labels. In +the spirit of "Everyone can contribute", any issue can be picked up by any group, +depending on current priorities. When picking up an issue belonging to a different +group, it should be relabelled. For example, if an issue labelled ~"devops::create" +and ~"group::knowledge" is picked up by someone in the Access group of the Plan stage, +the issue should be relabelled as ~"group::access" while keeping the original +~"devops::create" unchanged. -We also use stage and group labels to help quantify our [throughput](https://about.gitlab.com/handbook/engineering/management/throughput). +We also use stage and group labels to help quantify our [throughput](https://about.gitlab.com/handbook/engineering/management/throughput/). Please read [Stage and Group labels in Throughtput](https://about.gitlab.com/handbook/engineering/management/throughput/#stage-and-group-labels-in-throughput) for more information on how the labels are used in this context. ### Category labels @@ -228,7 +235,7 @@ people. The current team labels are: - ~Delivery -- ~Documentation +- ~"Technical Writing" #### Naming and color convention @@ -241,6 +248,7 @@ These labels narrow the [specialization](https://about.gitlab.com/company/team/s - ~frontend - ~backend +- ~documentation ### Release scoping labels @@ -334,7 +342,7 @@ know how difficult the issue is. Additionally: - We advertise [`Accepting merge requests` issues with weight < 5](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight) as suitable for people that have never contributed to GitLab before on the - [Up For Grabs campaign](http://up-for-grabs.net) + [Up For Grabs campaign](https://up-for-grabs.net/#/) - We encourage people that have never contributed to any open source project to look for [`Accepting merge requests` issues with a weight of 1](https://gitlab.com/groups/gitlab-org/-/issues?state=opened&label_name[]=Accepting+merge+requests&assignee_id=None&sort=weight&weight=1) diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 1931dda7151..86f17f4ecdb 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -89,7 +89,7 @@ request is as follows: 1. Write tests for more complex migrations. 1. Merge requests **must** adhere to the [merge request performance guidelines](../merge_request_performance_guidelines.md). 1. For tests that use Capybara, read - [how to write reliable, asynchronous integration tests](https://robots.thoughtbot.com/write-reliable-asynchronous-integration-tests-with-capybara). + [how to write reliable, asynchronous integration tests](https://thoughtbot.com/blog/write-reliable-asynchronous-integration-tests-with-capybara). 1. If your merge request introduces changes that require additional steps when installing GitLab from source, add them to `doc/install/installation.md` in the same merge request. @@ -101,7 +101,7 @@ request is as follows: If you would like quick feedback on your merge request feel free to mention someone from the [core team](https://about.gitlab.com/community/core-team/) or one of the -[merge request coaches](https://about.gitlab.com/team/). When having your code reviewed +[merge request coaches](https://about.gitlab.com/company/team/). When having your code reviewed and when reviewing merge requests, please keep the [code review guidelines](../code_review.md) in mind. And if your code also makes changes to the database, or does expensive queries, check the [database review guidelines](../database_review.md). @@ -140,7 +140,7 @@ When writing commit messages, please follow the guidelines below: - The merge request must not contain more than 10 commit messages. If the guidelines are not met, the MR will not pass the -[Danger checks](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/danger/commit_messages/Dangerfile). +[Danger checks](https://gitlab.com/gitlab-org/gitlab/blob/master/danger/commit_messages/Dangerfile). For more information see [How to Write a Git Commit Message](https://chris.beams.io/posts/git-commit/). Example commit message template that can be used on your machine that embodies the above (guide for [how to apply template](https://codeinthehole.com/tips/a-useful-template-for-commit-messages/)): @@ -220,6 +220,8 @@ requirements. 1. Working and clean code that is commented where needed. 1. [Unit, integration, and system tests](../testing_guide/index.md) that all pass on the CI server. +1. Regressions and bugs are covered with tests that reduce the risk of the issue happening + again. 1. Performance/scalability implications have been considered, addressed, and tested. 1. [Documented](../documentation/index.md) in the `/doc` directory. 1. [Changelog entry added](../changelog.md), if necessary. @@ -244,5 +246,5 @@ request: 1. [The upgrade guide](../../update/upgrading_from_source.md). 1. The [GitLab Installation Guide](../../install/installation.md#1-packages-and-dependencies). 1. The [GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit). -1. The [CI environment preparation](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/scripts/prepare_build.sh). +1. The [CI environment preparation](https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/prepare_build.sh). 1. The [Omnibus package creator](https://gitlab.com/gitlab-org/omnibus-gitlab). diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index a6650a50878..0b2e2b43512 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -15,7 +15,7 @@ to the existing rules, then this is the document for you. ## Operation -On startup, Danger reads a [`Dangerfile`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/Dangerfile) +On startup, Danger reads a [`Dangerfile`](https://gitlab.com/gitlab-org/gitlab/blob/master/Dangerfile) from the project root. GitLab's Danger code is decomposed into a set of helpers and plugins, all within the [`danger/`](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/danger/) subdirectory, so ours just tells Danger to load it all. Danger will then run diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index 6c9fa983c96..9947f9c16c0 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -3,7 +3,7 @@ This section is to help give some copy-pasta you can use as a reference when you run into some head-banging database problems. -An easy first step is to search for your error in Slack or google "GitLab (my error)". +An easy first step is to search for your error in Slack, or search for `GitLab <my error>` with Google. --- diff --git a/doc/development/database_review.md b/doc/development/database_review.md index 57cdc2135aa..39236ab1910 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -78,7 +78,8 @@ and details for a database reviewer: - Format any queries with a SQL query formatter, for example with [sqlformat.darold.net](http://sqlformat.darold.net). - Consider providing query plans via a link to [explain.depesz.com](https://explain.depesz.com) or another tool instead of textual form. - For query changes, it is best to provide the SQL query along with a plan *before* and *after* the change. This helps to spot differences quickly. -- When providing query plans, make sure to use good parameter values, so that the query executed is a good example and also hits enough data. Usually, the `gitlab-org` namespace (`namespace_id = 9970`) and the `gitlab-org/gitlab-ce` project (`project_id = 13083`) provides enough data to serve as a good example. +- When providing query plans, make sure to use good parameter values, so that the query executed is a good example and also hits enough data. + - Usually, the `gitlab-org` namespace (`namespace_id = 9970`) and the `gitlab-org/gitlab-foss` (`project_id = 13083`) or the `gitlab-org/gitlab` (`project_id = 278964`) projects provide enough data to serve as a good example. ### How to review for database @@ -94,17 +95,22 @@ and details for a database reviewer: - Check queries timing (If any): Queries executed in a migration need to fit comfortably within `15s` - preferably much less than that - on GitLab.com. - Check [background migrations](background_migrations.md): - - For data migrations, establish a time estimate for execution + - Establish a time estimate for execution on GitLab.com. - They should only be used when migrating data in larger tables. - If a single `update` is below than `1s` the query can be placed directly in a regular migration (inside `db/migrate`). - Review queries (for example, make sure batch sizes are fine) - - Establish a time estimate for execution - Because execution time can be longer than for a regular migration, it's suggested to treat background migrations as post migrations: place them in `db/post_migrate` instead of `db/migrate`. Keep in mind that post migrations are executed post-deployment in production. - Check [timing guidelines for migrations](#timing-guidelines-for-migrations) +- Check migrations are reversible and implement a `#down` method +- Check data migrations: + - Establish a time estimate for execution on GitLab.com. + - Depending on timing, data migrations can be placed on regular, post-deploy or background migrations. + - Data migrations should be reversible too or come with a description of how to reverse, when possible. + This applies to all types of migrations (regular, post-deploy, background). - Query performance - Check for any obviously complex queries and queries the author specifically points out for review (if any) @@ -120,7 +126,7 @@ and details for a database reviewer: pipeline](https://ops.gitlab.net/gitlab-com/gl-infra/gitlab-restore/postgres-gprd) in order to establish a proper testing environment. -### Timing guidelines for migrations +### Timing guidelines for migrations In general, migrations for a single deploy shouldn't take longer than 1 hour for GitLab.com. The following guidelines are not hard rules, they were diff --git a/doc/development/deleting_migrations.md b/doc/development/deleting_migrations.md new file mode 100644 index 00000000000..438e8c9f5e9 --- /dev/null +++ b/doc/development/deleting_migrations.md @@ -0,0 +1,33 @@ +# Delete existing migrations + +When removing existing migrations from the GitLab project, you have to take into account +the possibility of the migration already been included in past releases or in the current release, and thus already executed on GitLab.com and/or in self-hosted instances. + +Because of it, it's not possible to delete existing migrations, as that could lead to: + +- Schema inconsistency, as changes introduced into the database were not rollbacked properly. +- Leaving a record on the `schema_versions` table, that points out to migration that no longer exists on the codebase. + +Instead of deleting we can opt for disabling the migration. + +## Pre-requisites to disable a migration + +Migrations can be disabled if: + +- They caused a timeout or general issue on GitLab.com. +- They are obsoleted, e.g. changes are not necessary due to a feature change. +- Migration is a data migration only, i.e. the migration does not change the database schema. + +## How to disable a data migration? + +In order to disable a migration, the following steps apply to all types of migrations: + +1. Turn the migration into a noop by removing the code inside `#up`, `#down` + or `#perform` methods, and adding `#no-op` comment instead. +1. Add a comment explaining why the code is gone. + +Disabling migrations requires explicit approval of Database Maintainer. + +## Examples + +- [Disable scheduling of productivity analytics](https://gitlab.com/gitlab-org/gitlab/merge_requests/17253) diff --git a/doc/development/documentation/feature-change-workflow.md b/doc/development/documentation/feature-change-workflow.md index 4512b0fc987..004d8833e63 100644 --- a/doc/development/documentation/feature-change-workflow.md +++ b/doc/development/documentation/feature-change-workflow.md @@ -1,178 +1,5 @@ --- -description: How to add docs for new or enhanced GitLab features. +redirect_to: 'workflow.md' --- -# Documentation process for feature changes - -At GitLab, developers contribute new or updated documentation along with their code, but product managers and technical writers also have essential roles in the process. - -- **Developers**: Author/update documentation in the same MR as their code, and - merge it by the feature freeze for the assigned milestone. Request technical writer - assistance if needed. Other developers typically act as reviewers. -- **Product Managers** (PMs): In the issue for all new and enhanced features, - confirm the documentation requirements, plus the mentioned feature description - and use cases, which can be reused in docs. They can bring in a technical - writer for discussion or collaboration, and can be called upon themselves as a doc reviewer. -- **Technical Writers**: Review doc requirements in issues, track issues and MRs - that contain docs changes, help with any questions throughout the authoring/editing process, - work on special projects related to the documentation, and review all new and updated - docs content, whether before or after it is merged. - -Beyond this process, any member of the GitLab community can also author or request documentation -improvements that are not associated with a new or changed feature. See the [Documentation improvement workflow](improvement-workflow.md). - -## When documentation is required - -Documentation must be delivered whenever: - -- A new or enhanced feature is shipped that impacts the user/admin experience. -- There are changes to the UI or API. -- A process, workflow, or previously documented feature is changed. -- A feature is deprecated or removed. - -For example, a UI restyling that offers no difference in functionality may require -documentation updates if screenshots are now needed, or need to be updated. - -Documentation is not required when a feature is changed on the backend -only and does not directly affect the way that any user or administrator would -interact with GitLab. - -NOTE: **Note:** -When revamping documentation, if unrelated to the feature change, this should be submitted -in its own MR (using the [documentation improvement workflow](improvement-workflow.md)) -so that we can ensure the more time-sensitive doc updates are merged with code by the freeze. - -## Documentation requirements in feature issues - -Requirements for the documentation of a feature should be included as part of the -issue for planning that feature, in a Documentation section within the issue description. - -This section is provided as part of the Feature Proposal template and should be added -to the issue if it is not already present. - -Anyone can add these details, but the product manager who assigns the issue to a specific release -milestone will ensure these details are present and finalized by the time of that milestone's kickoff. -Developers, technical writers, and others may help further refine this plan at any time. - -### Details to include - -- What concepts and procedures should the docs guide and enable the user to understand or accomplish? -- To this end, what new page(s) are needed, if any? What pages/subsections need updates? Consider user, admin, and API doc changes and additions. -- For any guide or instruction set, should it help address a single use case, or be flexible to address a certain range of use cases? -- Do we need to update a previously recommended workflow? Should we link the new feature from various relevant locations? Consider all ways documentation should be affected. -- Are there any key terms or task descriptions that should be included so that the docs are found in relevant searches? -- Include suggested titles of any pages or subsections, if applicable. - -## Documenting a new or changed feature - -To follow a consistent workflow every month, documentation changes -involve the Product Managers, the developer who shipped the feature, -and the technical writer for the DevOps stage. Each role is described below. - -The Documentation items in the GitLab CE/EE [Feature Proposal issue template](https://gitlab.com/gitlab-org/gitlab/raw/master/.gitlab/issue_templates/Feature%20proposal.md) -and default merge request template will assist you with following this process. - -### Product Manager role - -For issues requiring any new or updated documentation, the Product Manager (PM) -must: - -- Add the `Documentation` label. -- Confirm or add the [documentation requirements](#documentation-requirements-in-feature-issues). -- Ensure the issue contains any new or updated feature name, overview/description, - and use cases, as required per the [documentation structure and template](structure.md), when applicable. - -Everyone is encouraged to draft the requirements in the issue, but a product manager will -do the following: - -- When the issue is assigned a release milestone, review and update the Documentation details. -- By the kickoff, finalize the Documentation details. - -### Developer and maintainer roles - -#### Authoring - -As a developer, if a ~feature issue also contains the ~Documentation label, you must ship the new or updated documentation with the code of the feature. The documentation is an essential part of the product. -Technical writers are happy to help, as requested and planned on an issue-by-issue basis. - -For feature issues requiring documentation, follow the process below unless otherwise agreed with the product manager and technical writer for a given issue: - -- Include any new and edited docs in the MR introducing the code. -- Use the Documentation requirements confirmed by the Product Manager in the - issue and discuss any further doc plans or ideas as needed. - - If the new or changed doc requires extensive collaboration or conversation, a separate, - linked issue can be used for the planning process. - - We are trying to avoid using a separate MR, so that the docs stay with the code, but the - Technical Writing team is interested in discussing any potential exceptions that may be suggested. -- Use the [Documentation guidelines](index.md), as well as other resources linked from there, - including the Documentation [Structure and template](structure.md) page, [Style Guide](styleguide.md), and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). -- If you need any help to choose the correct place for a doc, discuss a documentation - idea or outline, or request any other help, ping the Technical Writer for the relevant - [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) - in your issue or MR, or write within `#docs` on the GitLab Slack. -- If you are working on documentation in a separate MR, ensure that if the code is merged by the 17th, the docs are as well, per the [Engineering Workflow](https://about.gitlab.com/handbook/engineering/workflow/). If the docs are not ready, the PM can approve merging the code if the engineer and tech writer commit to get documentation merged by the 21st. Otherwise the feature is not considered complete, and should not be merged. -- A policy for documenting feature-flagged - issues is forthcoming and you are welcome to join the [discussion](https://gitlab.com/gitlab-org/gitlab-foss/issues/56813). - -#### Reviews and merging - -All reviewers can help ensure accuracy, clarity, completeness, and adherence to the plans in the issue, as well as the [Documentation Guidelines](index.md) and [Style Guide](styleguide.md). - -- **Prior to merging**, documentation changes committed by the developer must be reviewed by: - - 1. **The code reviewer** for the MR, to confirm accuracy, clarity, and completeness. - 1. Optionally: Others involved in the work, such as other devs or the PM. - 1. Optionally: The technical writer for the DevOps stage. If not prior to merging, the technical writer will review after the merge. - This helps us ensure that the developer has time to merge good content by the freeze, and that it can be further refined by the release, if needed. - - To decide whether to request this review before the merge, consider the amount of time left before the code freeze, the size of the change, - and your degree of confidence in having users of an RC use your docs as written. - - Pre-merge tech writer reviews should be most common when the code is complete well in advance of the freeze and/or for larger documentation changes. - - You can request a review and if there is not sufficient time to complete it prior to the freeze, - the maintainer can merge the current doc changes (if complete) and create a follow-up doc review issue. - - The technical writer can also help decide what docs to merge before the freeze and whether to work on further changes in a follow up MR. - - **To request a pre-merge technical writer review**, assign the writer listed for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). - - **To request a post-merge technical writer review**, [create an issue for one using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-foss/issues/new?issuable_template=Doc%20Review) and link it from the MR that makes the doc change. - 1. **The maintainer** who is assigned to merge the MR, to verify clarity, completeness, and quality, to the best of their ability. - -- Upon merging, if a technical writer review has not been performed and there is not yet a linked issue for a follow-up review, the maintainer should [create an issue using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-foss/issues/new?issuable_template=Doc%20Review), link it from the MR, and - mention the original MR author in the new issue. Alternatively, the maintainer can ask the MR author to create and link this issue before the MR is merged. - -- After merging, documentation changes are reviewed by: - - 1. The technical writer -- **if** their review was not performed prior to the merge. - 1. Optionally: by the PM (for accuracy and to ensure it's consistent with the vision for how the product will be used). - Any party can raise the item to the PM for review at any point: the dev, the technical writer, or the PM, who can request/plan a review at the outset. - -### Technical Writer role - -#### Planning - -- The technical writer monitors the documentation needs of issues assigned to the current and next milestone - for their DevOps stage(s), and participates in any needed discussion on docs planning and requirements refinement - with the dev, PM, and others. -- The technical writer will review these requirements again upon the kickoff and provide feedback, as needed. - This is not a blocking review and developers should not wait to work on docs. - -#### Collaboration - -By default, the developer will work on documentation changes independently, but -the developer, PM, or technical writer can propose a broader collaboration for -any given issue. - -Additionally, technical writers are available for questions at any time. - -#### Review - -- Technical writers provide non-blocking reviews of all documentation changes, - before or after the change is merged. However, if the docs are ready in the MR while - there's time before the freeze, the technical writer's review can commence early, on request. -- The technical writer will confirm that the doc is clear, grammatically correct, - and discoverable, while avoiding redundancy, bad file locations, typos, broken links, - etc. The technical writer will review the documentation for the following, which - the developer and code reviewer should have already made a good-faith effort to ensure: - - Clarity. - - Adherence to the plans and goals in the issue. - - Location (make sure the docs are in the correct directories and has the correct name). - - Syntax, typos, and broken links. - - Improvements to the content. - - Accordance with the [Documentation Style Guide](styleguide.md), and [Structure and Template](structure.md) doc. +This document was moved to [another location](workflow.md). diff --git a/doc/development/documentation/improvement-workflow.md b/doc/development/documentation/improvement-workflow.md index 9f3b789712f..004d8833e63 100644 --- a/doc/development/documentation/improvement-workflow.md +++ b/doc/development/documentation/improvement-workflow.md @@ -1,63 +1,5 @@ --- -description: How to improve GitLab's documentation. +redirect_to: 'workflow.md' --- -# Documentation improvement workflow - -Anyone can contribute a merge request or create an issue for GitLab's documentation. - -This page covers the process for any contributions to GitLab's docs that are -not part of feature development. If you are looking for information on updating -GitLab's docs as is required with the development and release of a new feature -or feature enhancement, see the [documentation process for feature changes](feature-change-workflow.md). - -## Who updates the docs - -Anyone can contribute! You can create a merge request with documentation -when you find errors or other room for improvement in an existing doc, or when you -have an idea for all-new documentation that would help a GitLab user or administrator -to accomplish their work with GitLab. - -## How to update the docs - -1. Click "Edit this Page" at the bottom of any page on docs.gitlab.com, or navigate to - one of the repositories and doc paths listed on the [GitLab Documentation guidelines](index.md) page. - Work in a fork if you do not have developer access to the GitLab project. -1. Follow the described standards and processes listed on that Guidelines page, - including the linked resources: the [Structure and template](structure.md) page, [Style Guide](styleguide.md), and [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). -1. Follow GitLab's [Merge Request Guidelines](../contributing/merge_request_workflow.md#merge-request-guidelines). - -If you need any help to choose the correct place for a doc, discuss a documentation -idea or outline, or request any other help, ping the Technical Writer for the relevant -[DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages) -in your issue or MR, or write within `#docs` if you are a member of GitLab's Slack workspace. - -## Review and merging - -Anyone with master access to the affected GitLab project can merge documentation changes. -This person must make a good-faith effort to ensure that the content is clear -(sufficiently easy for the intended audience to navigate and understand) and -that it meets the [Documentation Guidelines](index.md) and [Style Guide](styleguide.md). - -If the author or reviewer has any questions, or would like a techncial writer's review -before merging, mention the writer who is assigned to the relevant [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). - -The process can involve the following parties/phases, and is replicated in the `Documentation` MR template for GitLab CE and EE, to help with following the process. - -**1. Primary Reviewer** - Review by a [code reviewer](https://about.gitlab.com/handbook/engineering/projects/) or other appropriate colleague to confirm accuracy, clarity, and completeness. This can be skipped for minor fixes without substantive content changes. - -**2. Technical Writer** - Optional - If not requested for this MR, must be scheduled post-merge. To request a pre-merge review, assign the writer listed for the applicable [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). -To request a post-merge review, [create an issue for one using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-foss/issues/new?issuable_template=Doc%20Review) and link it from the MR that makes the doc change. - -**3. Maintainer** - -1. Review by assigned maintainer, who can always request/require the above reviews. Maintainer review can occur before or after a technical writer review. -1. Ensure a release milestone of the format XX.Y is set. If the freeze for that release has begun, add the label `pick into <XX.Y>` unless this change is not required for the release. In that case, simply change the milestone. -1. If EE and CE MRs exist, merge the EE MR first, then the CE MR. -1. After merging, if there has not been a technical writer review and an issue for a follow-up review was not already created and linked from the MR, [create the issue using the Doc Review template](https://gitlab.com/gitlab-org/gitlab-foss/issues/new?issuable_template=Doc%20Review) and link it from the MR. - -## Other ways to help - -If you have ideas for further documentation resources that would be best -considered/handled by technical writers, devs, and other SMEs, please [create an issue](https://gitlab.com/gitlab-org/gitlab-foss/issues/new?issuable_template=Documentation) -using the Documentation template. +This document was moved to [another location](workflow.md). diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 4ad24d08d17..fb0aa5130f8 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -8,19 +8,16 @@ GitLab's documentation is [intended as the single source of truth (SSOT)](https: In addition to this page, the following resources can help you craft and contribute documentation: -- [Style Guide](styleguide.md) - What belongs in the docs, language guidelines, and more. +- [Style Guide](styleguide.md) - What belongs in the docs, language guidelines, Markdown standards to follow, and more. - [Structure and template](structure.md) - Learn the typical parts of a doc page and how to write each one. -- [Workflows](workflow.md) - A landing page for our key workflows: - - [Documentation process for feature changes](feature-change-workflow.md) - Adding required documentation when developing a GitLab feature. - - [Documentation improvement workflow](improvement-workflow.md) - New content not associated with a new feature. -- [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) - A reference for the markdown implementation used by GitLab's documentation site and about.gitlab.com. -- [Site architecture](site_architecture/index.md) - How docs.gitlab.com is built. +- [Documentation process](workflow.md). +- [Markdown Guide](../../user/markdown.md) - A reference for all Markdown syntax supported by GitLab. +- [Site architecture](site_architecture/index.md) - How <https://docs.gitlab.com> is built. ## Source files and rendered web locations -Documentation for GitLab, GitLab Runner, and Omnibus is published to [docs.gitlab.com](https://docs.gitlab.com). Documentation for GitLab is also published within the application at `/help` on the domain of the GitLab instance. - -At `/help`, only help for your current edition and version is included. Help for other versions is available at docs.gitlab.com. +Documentation for GitLab, GitLab Runner, Omnibus GitLab and Charts are published to <https://docs.gitlab.com>. Documentation for GitLab is also published within the application at `/help` on the domain of the GitLab instance. +At `/help`, only help for your current edition and version is included. Help for other versions is available at <https://docs.gitlab.com/archives/>. The source of the documentation exists within the codebase of each GitLab application in the following repository locations: @@ -29,6 +26,7 @@ The source of the documentation exists within the codebase of each GitLab applic | [GitLab](https://gitlab.com/gitlab-org/gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab/tree/master/doc) | | [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/) | [`/docs`](https://gitlab.com/gitlab-org/gitlab-runner/tree/master/docs) | | [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab/tree/master/doc) | +| [Charts](https://gitlab.com/gitlab-org/charts/gitlab) | [`/doc`](https://gitlab.com/gitlab-org/charts/gitlab/tree/master/doc) | Documentation issues and merge requests are part of their respective repositories and all have the label `Documentation`. @@ -55,8 +53,8 @@ See the [Structure](styleguide.md#structure) section of the [Documentation Style Changing a document's location requires specific steps to ensure that users can seamlessly access the new doc page, whether they are accessing content -on a GitLab instance domain at `/help` or at docs.gitlab.com. Be sure to ping a -GitLab technical writer if you have any questions during the process (such as +on a GitLab instance domain at `/help` or at <https://docs.gitlab.com>. Be sure to assign a +technical writer if you have any questions during the process (such as whether the move is necessary), and ensure that a technical writer reviews this change prior to merging. @@ -132,7 +130,7 @@ land on the doc via `/help`. If the documentation page being relocated already has Disqus comments, we need to preserve the Disqus thread. -Disqus uses an identifier per page, and for docs.gitlab.com, the page identifier +Disqus uses an identifier per page, and for <https://docs.gitlab.com>, the page identifier is configured to be the page URL. Therefore, when we change the document location, we need to preserve the old URL as the same Disqus identifier. @@ -181,9 +179,9 @@ Every GitLab instance includes the documentation, which is available at `/help` (`https://gitlab.example.com/help`). For example, <https://gitlab.com/help>. There are [plans](https://gitlab.com/groups/gitlab-org/-/epics/693) to end this -practice and instead link out from the GitLab application to docs.gitlab.com URLs. +practice and instead link out from the GitLab application to <https://docs.gitlab.com> URLs. -The documentation available online on docs.gitlab.com is continuously +The documentation available online on <https://docs.gitlab.com> is continuously deployed every hour from the `master` branch of GitLab, Omnibus, and Runner. Therefore, once a merge request gets merged, it will be available online on the same day. However, they will be shipped (and available on `/help`) within the milestone assigned @@ -195,7 +193,7 @@ available online on 2018-09-15, but, as the feature freeze date has passed, if the MR does not have a "pick into 11.3" label, the milestone has to be changed to 11.4 and it will be shipped with all GitLab packages only on 2018-10-22, with GitLab 11.4. Meaning, it will only be available under `/help` from GitLab -11.4 onwards, but available on docs.gitlab.com on the same day it was merged. +11.4 onwards, but available on <https://docs.gitlab.com/> on the same day it was merged. ### Linking to `/help` @@ -271,7 +269,7 @@ For example, [GitLab.com's `/help`](https://gitlab.com/help). ## Docs site architecture See the [Docs site architecture](site_architecture/index.md) page to learn -how we build and deploy the site at [docs.gitlab.com](https://docs.gitlab.com) and +how we build and deploy the site at <https://docs.gitlab.com> and to review all the assets and libraries in use. ### Global navigation @@ -301,18 +299,18 @@ 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-org/gitlab-docs) - 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: +1. Create a new branch in the [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) + project named after the scheme: `docs-preview-$DOCS_GITLAB_REPO_SUFFIX-$CI_MERGE_REQUEST_IID`, + where `DOCS_GITLAB_REPO_SUFFIX` is the suffix for each product, e.g, `ee` for + EE, `omnibus` for Omnibus GitLab, etc, and `CI_MERGE_REQUEST_IID` is the ID + of the respective merge request. +1. Trigger a cross project pipeline and build the docs site with your changes. -- In the merge request widget -- 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, this 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 can check the status of the +remote pipeline from the link in the merge request's job output. +If the pipeline failed or got stuck, drop a line in the `#docs` chat channel. TIP: **Tip:** Someone with no merge rights to the GitLab projects (think of forks from @@ -345,12 +343,11 @@ If you want to know the in-depth details, here's what's really happening: 1. The job runs the [`scripts/trigger-build-docs`](https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/trigger-build-docs) script with the `deploy` flag, which in turn: 1. Takes your branch name and applies the following: - - The slug of the branch name is used to avoid special characters since - ultimately this will be used by NGINX. - - The `preview-` prefix is added to avoid conflicts if there's a remote branch - with the same name that you created in the merge request. - - The final branch name is truncated to 42 characters to avoid filesystem - limitations with long branch names (> 63 chars). + - The `docs-preview-` prefix is added. + - The product slug is used to know the project the review app originated + from. + - The number of the merge request is added so that you can know by the + `gitlab-docs` branch name the merge request it originated from. 1. The remote branch is then created if it doesn't exist (meaning you can re-run the manual job as many times as you want and this step will be skipped). 1. A new cross-project pipeline is triggered in the docs project. @@ -371,20 +368,33 @@ The following GitLab features are used among others: - [Review Apps](../../ci/review_apps/index.md) - [Artifacts](../../ci/yaml/README.md#artifacts) - [Specific Runner](../../ci/runners/README.md#locking-a-specific-runner-from-being-enabled-for-other-projects) +- [Pipelines for merge requests](../../ci/merge_request_pipelines/index.md) ## Testing -We treat documentation as code, and so use tests to maintain the standards and quality of the docs. -The current tests are: - -1. `docs lint`: Check that all internal (relative) links work correctly and - that all cURL examples in API docs use the full switches. It's recommended - to [check locally](#previewing-the-changes-live) before pushing to GitLab by executing the command - `bundle exec nanoc check internal_links` on your local - [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) directory. In addition, - `docs-lint` also runs [`markdownlint`](#markdownlint) to ensure the - markdown is consistent across all documentation. -1. In a full pipeline, tests for [`/help`](#gitlab-help-tests). +We treat documentation as code, and so use tests in our CI pipeline to maintain the +standards and quality of the docs. The current tests, which run in CI jobs when a +merge request with new or changed docs is submitted, are: + +- [`docs lint`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L48): + Runs several tests on the content of the docs themselves: + - [`lint-doc.sh` script](https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/lint-doc.sh) + checks that: + - All cURL examples use the long flags (ex: `--header`, not `-H`). + - The `CHANGELOG.md` does not contain duplicate versions. + - No files in `doc/` are executable. + - No new `README.md` was added. + - [`markdownlint`](#markdownlint). + - Nanoc tests, which you can [run locally](#previewing-the-changes-live) before + pushing to GitLab by executing the command `bundle exec nanoc check internal_links` + (or `internal_anchors`) on your local [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs) directory: + - [`internal_links`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L67) + checks that all internal links (ex: `[link](../index.md)`) are valid. + - [`internal_anchors`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/docs.gitlab-ci.yml#L69) + checks that all internal anchors (ex: `[link](../index.md#internal_anchor)`) + are valid. +- If any code or the `doc/README.md` file is changed, a full pipeline will run, which + runs tests for [`/help`](#gitlab-help-tests). ### Linting @@ -490,7 +500,10 @@ four repos that are the sources for <https://docs.gitlab.com>: - <https://gitlab.com/charts/gitlab/blob/master/.markdownlint.json> By default all rules are enabled, so the configuration file is used to disable unwanted -rules, and also to configure optional parameters for enabled rules as needed. +rules, and also to configure optional parameters for enabled rules as needed. You can +also check [the issue](https://gitlab.com/gitlab-org/gitlab-foss/issues/64352) that +tracked the changes required to implement these rules, and details which rules were +on or off when `markdownlint` was enabled on the docs. ## Danger Bot diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index bb598906967..f5a12e9c216 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -18,8 +18,8 @@ from where content is sourced, the `gitlab-docs` project, and the published outp ```mermaid graph LR - A[gitlab-ce/doc] - B[gitlab-ee/doc] + A[gitlab-foss/doc] + B[gitlab/doc] C[gitlab-runner/docs] D[omnibus-gitlab/doc] E[charts/doc] @@ -68,7 +68,7 @@ the GitLab Documentation website. ## Global navigation -Read through the global navigation](global_nav.md) documentation to understand: +Read through [the global navigation documentation](global_nav.md) to understand: - How the global navigation is built. - How to add new navigation items. diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index 158e69df2a6..21219dc100a 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -32,7 +32,7 @@ For additional details on each, see the [template for new docs](#template-for-ne below. Note that you can include additional subsections, as appropriate, such as 'How it Works', 'Architecture', -and other logicial divisions such as pre- and post-deployment steps. +and other logical divisions such as pre- and post-deployment steps. ## Template for new docs diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index 79797107a5b..b6ec7a858fa 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -212,15 +212,36 @@ Do not include the same information in multiple places. [Link to a SSOT instead. - Use inclusive language and avoid jargon, as well as uncommon words. The docs should be clear and easy to understand. -- Write in the 3rd person (use "we", "you", "us", "one", instead of "I" or "me"). +- Write in the 3rd person (use "we," "you," "us," "one," instead of "I" or "me"). - Be clear, concise, and stick to the goal of the doc. -- Write in US English. +- Write in US English with US grammar. - Capitalize "G" and "L" in GitLab. -- 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. "GitLab's Merge Requests support X." and "Create a new merge request for Z."). +- Use title case when referring to: + - [GitLab Features](https://about.gitlab.com/features/). For example, Issue Board, + Geo, and Runner. + - GitLab [product tiers](https://about.gitlab.com/pricing/). For example, GitLab Core + and GitLab Ultimate. + - Third-party products. For example, Prometheus, Kubernetes, and Git. + - Methods or methodologies. For example, Continuous Integration, Continuous + Deployment, Scrum, and Agile. + + NOTE: **Note:** + Some features are also objects. For example, "GitLab's Merge Requests support X" and + "Create a new merge request for Z." + +- Avoid use of the future tense: + - Instead of, "After you execute this command, the result will be displayed," say "After you execute this command, the result is displayed." + - Only use the future tense to convey when the action or result will actually occur at a future time. +- Do not use contractions: + - Instead of "don't," "can't," "doesn't," and so on, say "do not," "cannot," or "does not." + - Possible exceptions are cases when a more familiar tone is desired, such as a blog post or other casual context. +- Do not use slashes to clump different words together or as a replacement for the word "or": + - Instead of "and/or," consider saying "or," or use another sensible construction. + - Other examples include "clone/fetch," author/assignee," and "namespace/repository name." Break apart any such instances in an appropriate way. + - Exceptions to this rule include commonly accepted technical terms such as CI/CD, TCP/IP, and so on. +- Do not use "may" and "might" interchangeably: + - Use "might" to indicate the probability of something occurring. "If you skip this step, the import process might fail." + - Use "may" to indicate giving permission for someone to do something, or consider using "can" instead. "You may select either option on this screen." Or, "you can select either option on this screen." ## Text @@ -239,27 +260,13 @@ Do not include the same information in multiple places. [Link to a SSOT instead. - List item 2 ``` -### Tables overlapping the TOC - -By default, all tables have a width of 100% on docs.gitlab.com. -In a few cases, the table will overlap the table of contents (ToC). -For these cases, add an entry to the document's frontmatter to -render them displaying block. This will make sure the table -is displayed behind the ToC, scrolling horizontally: - -```md ---- -table_display_block: true ---- -``` - -## Emphasis +### Emphasis - Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). - Use underscore (`_`) for text in italics (`_italic_`). - Use greater than (`>`) for blockquotes. -## Punctuation +### Punctuation Check the general punctuation rules for the GitLab documentation on the table below. Check specific punctuation rules for [lists](#lists) below. @@ -274,6 +281,20 @@ Check specific punctuation rules for [lists](#lists) below. | Always add a space before and after dashes when using it in a sentence (for replacing a comma, for example). | _You should try this - or not._ | | Always use lowercase after a colon. | _Related Issues: a way to create a relationship between issues._ | +### Placeholder text + +Often in examples, a writer will provide a command or configuration that is complete apart from +a value specific to the reader. + +In these cases, use [`<` and `>`](https://en.wikipedia.org/wiki/Usage_message#Pattern) to call out +where a reader must replace text with their own value. + +For example: + +```sh +cp <your_source_directory> <your_destination_directory> +``` + ## Lists - Always start list items with a capital letter, unless they are parameters or commands @@ -463,24 +484,58 @@ For other punctuation rules, please refer to the - Leave exactly one blank line before and after a heading. - Do not use links in headings. - Add the corresponding [product badge](#product-badges) according to the tier the feature belongs. +- Use sentence case in headings. Do not capitalize the words of the title, unless + it refers to a product feature. For example, capitalizing "issues" is acceptable in + `## What you can do with GitLab Issues`, but not in `## Closing multiple issues`. ## Links - Use inline link markdown markup `[Text](https://example.com)`. It's easier to read, review, and maintain. **Do not** use `[Text][identifier]`. -- To link to internal documentation, use relative links, not full URLs. Use `../` to - navigate to high-level directories, and always add the file name `file.md` at the - end of the link with the `.md` extension, not `.html`. - Example: instead of `[text](../../merge_requests/)`, use - `[text](../../merge_requests/index.md)` or, `[text](../../ci/README.md)`, or, - for anchor links, `[text](../../ci/README.md#examples)`. - Using the markdown extension is necessary for the [`/help`](index.md#gitlab-help) - section of GitLab. -- To link from CE to EE-only documentation, use the EE-only doc full URL. + - Use [meaningful anchor texts](https://www.futurehosting.com/blog/links-should-have-meaningful-anchor-text-heres-why/). E.g., instead of writing something like `Read more about GitLab Issue Boards [here](LINK)`, write `Read more about [GitLab Issue Boards](LINK)`. +### Links to internal documentation + +- To link to internal documentation, use relative links, not full URLs. + Use `../` to navigate to high-level directories. Links should not refer to root. + + Don't: + + ```md + [Geo Troubleshooting](https://docs.gitlab.com/ee/administration/geo/replication/troubleshooting.html) + [Geo Troubleshooting](/ee/administration/geo/replication/troubleshooting.md) + ``` + + Do: + + ```md + [Geo Troubleshooting](../../geo/replication/troubleshooting.md) + ``` + +- Always add the file name `file.md` at the end of the link with the `.md` extension, not `.html`. + + Don't: + + ```md + [merge requests](../../merge_requests/) + [issues](../../issues/tags.html) + [issue tags](../../issues/tags.html#stages) + ``` + + Do: + + ```md + [merge requests](../../merge_requests/index.md) + [issues](../../issues/tags.md) + [issue tags](../../issues/tags.md#stages) + ``` + +- Using the markdown extension is necessary for the [`/help`](index.md#gitlab-help) + section of GitLab. + ### Links requiring permissions Don't link directly to: @@ -535,7 +590,7 @@ To indicate the steps of navigation through the UI: a valid name for an illustration is `devops_diagram_v11_1.png`. - Keep all file names in lower case. - Consider using PNG images instead of JPEG. -- Compress all images with <https://tinypng.com/> or similar tool. +- Compress all images with <https://pngquant.org/> or similar tool. - Compress gifs with <https://ezgif.com/optimize> or similar tool. - Images should be used (only when necessary) to _illustrate_ the description of a process, not to _replace_ it. @@ -557,7 +612,7 @@ Inside the document: ### Remove image shadow -All images displayed on docs.gitlab.com have a box shadow by default. +All images displayed on the [GitLab Docs site](https://docs.gitlab.com) have a box shadow by default. To remove the box shadow, use the image class `.image-noshadow` applied directly to an HTML `img` tag: @@ -591,7 +646,7 @@ You can link any up-to-date video that is useful to the GitLab user. > [Introduced](https://gitlab.com/gitlab-org/gitlab-docs/merge_requests/472) in GitLab 12.1. -GitLab docs (docs.gitlab.com) support embedded videos. +The [GitLab Docs site](https://docs.gitlab.com) supports embedded videos. You can only embed videos from [GitLab's official YouTube account](https://www.youtube.com/channel/UCnMGQ8QHMAnVIsI3xJrihhg). @@ -627,7 +682,7 @@ leave a blank line here leave a blank line here ``` -This is how it renders on docs.gitlab.com: +This is how it renders on the GitLab Docs site: <div class="video-fallback"> See the video: <a href="https://www.youtube.com/watch?v=enMumwvLAug">What is GitLab</a>. @@ -686,6 +741,10 @@ use the following markup for highlighting. _Note that the alert boxes only work for one paragraph only. Multiple paragraphs, lists, headers, etc will not render correctly. For multiple lines, use blockquotes instead._ +Alert boxes only render on the GitLab Docs site (<https://docs.gitlab.com>). +Within GitLab itself, they will appear as plain markdown text (like the examples +above the rendered versions, below). + ### Note Notes catch the eye of most readers, and therefore should be used very sparingly. @@ -710,7 +769,7 @@ NOTE: **Note:** This is something to note. ``` -How it renders in docs.gitlab.com: +How it renders on the GitLab Docs site: NOTE: **Note:** This is something to note. @@ -722,7 +781,7 @@ TIP: **Tip:** This is a tip. ``` -How it renders in docs.gitlab.com: +How it renders on the GitLab Docs site: TIP: **Tip:** This is a tip. @@ -734,7 +793,7 @@ CAUTION: **Caution:** This is something to be cautious about. ``` -How it renders in docs.gitlab.com: +How it renders on the GitLab Docs site: CAUTION: **Caution:** This is something to be cautious about. @@ -746,7 +805,7 @@ DANGER: **Danger:** This is a breaking change, a bug, or something very important to note. ``` -How it renders in docs.gitlab.com: +How it renders on the GitLab Docs site: DANGER: **Danger:** This is a breaking change, a bug, or something very important to note. @@ -759,7 +818,7 @@ For highlighting a text within a blue blockquote, use this format: > This is a blockquote. ``` -which renders in docs.gitlab.com to: +which renders on the [GitLab Docs site](https://docs.gitlab.com) as: > This is a blockquote. @@ -992,6 +1051,38 @@ In this case: - Different highlighting languages are used for each config in the code block. - The [GitLab Restart](#gitlab-restart) section is used to explain a required restart/reconfigure of GitLab. +## Feature flags + +Sometimes features are shipped with feature flags, either: + +- On by default, but providing the option to turn the feature off. +- Off by default, but providing the option to turn the feature on. + +When documenting feature flags for a feature, it's important that users know: + +- Why a feature flag is necessary. Some of the reasons are + [outlined in the handbook](https://about.gitlab.com/handbook/product/#alpha-beta-ga). +- That administrative access is required to make a feature flag change. +- What to ask for when requesting a change to a feature flag's state. + +NOTE: **Note:** +The [Product Manager for the relevant group](https://about.gitlab.com/handbook/product/categories/#devops-stages) +must review and approve the addition or removal of any mentions of using feature flags before the doc change is merged. + +The following is sample text for adding feature flag documentation for a feature: + +````md +### Disabling the feature + +This feature comes with the `:feature_flag` feature flag enabled by default. However, in some cases +this feature is incompatible with old configuration. To turn off the feature while configuration is +migrated, ask a GitLab administrator with Rails console access to run the following command: + +```ruby +Feature.disable(:feature_flag) +``` +```` + ## API Here is a list of must-have items. Use them in the exact order that appears @@ -1100,7 +1191,7 @@ Rendered example: ### cURL Examples -Below is a set of [cURL][] examples that you can use in the API documentation. +Below is a set of [cURL](https://curl.haxx.se) examples that you can use in the API documentation. #### Simple cURL command @@ -1147,7 +1238,7 @@ curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" --form "title= ``` The above example is run by and administrator and will add an SSH public key -titled ssh-key to user's account which has an id of 25. +titled `ssh-key` to user's account which has an id of 25. #### Escape special characters @@ -1172,7 +1263,6 @@ restrict the sign-up e-mail domains of a GitLab instance to `*.example.com` and curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" --data "domain_whitelist[]=*.example.com" --data "domain_whitelist[]=example.net" https://gitlab.example.com/api/v4/application/settings ``` -[cURL]: http://curl.haxx.se/ "cURL website" [single spaces]: http://www.slate.com/articles/technology/technology/2011/01/space_invaders.html [gfm]: ../../user/markdown.md#newlines "GitLab flavored markdown documentation" [ce-1242]: https://gitlab.com/gitlab-org/gitlab-foss/issues/1242 diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index 9f488fac7d0..24399391b1a 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -1,10 +1,332 @@ ---- -description: Learn the processes for contributing to GitLab's documentation. ---- +# Documentation process -# Documentation workflows +The process for creating and maintaining GitLab product documentation depends on whether the +documentation is associated with: -Documentation workflows at GitLab differ depending on the reason for the change: +- [A new feature or feature enhancement](#for-a-product-change). -- [Documentation process for feature changes](feature-change-workflow.md) - The documentation is being created or updated as part of the development and release of a new or enhanced feature. This process involves the developer of the feature (who includes new/updated documentation files as part of the same merge request containing the feature's code) and also involves the product manager and technical writer who are listed for the feature's [DevOps stage](https://about.gitlab.com/handbook/product/categories/#devops-stages). -- [Documentation improvement workflow](improvement-workflow.md) - All documentation additions not associated with a feature release. Documentation is being created or updated to improve accuracy, completeness, ease of use, or any reason other than a feature change. Anyone (and everyone) can contribute a merge request for this type of change at any time. + Delivered for a specific milestone and associated with specific code changes. This documentation + has the highest priority. + +- [Changes outside a specific milestone](#for-all-other-documentation). + + It is usually not associated with a specific code change and has a lower priority. + +Documentation is not usually required when a "backstage feature" is added or changed, and does not +directly affect the way that any user or administrator interacts with GitLab. + +## For a product change + +This documentation is required for any new or changed feature and is: + +- Created or updated as part of feature development, typically via the same merge request as the + feature code. +- Required with the delivery of a feature for a specific milestone as part of GitLab's + [definition of done](../contributing/merge_request_workflow.md#definition-of-done). +- Often linked from the release post. + +### Roles and responsibilities + +Documentation for specific milestones involves the: + +- Developer of a feature or enhancement. +- Product Manager for the group delivering the new feature or feature enhancement. +- Technical Writer assigned to the group. + +Each role is described below. + +#### Developers + +Developers are the primary author of documentation for a feature or feature enhancement. They are +responsible for: + +- Developing initial content required for a feature. +- Liaising with their Product Manager to understand what documentation must be delivered, and when. +- Requesting technical reviews from other developers within their group. +- Requesting documentation reviews from the Technical Writer + [assigned to the DevOps stage group](https://about.gitlab.com/handbook/product/technical-writing/index.html#assignments) + that is delivering the new feature or feature enhancements. + +TIP: **Tip:** +Community Contributors can ask for additional help from GitLab team members. + +##### Authoring + +Because the documentation is an essential part of the product, if a ~feature issue also contains the +~documentation label, you must ship the new or updated documentation with the code of the feature. + +Technical Writers are happy to help, as requested and planned on an issue-by-issue basis. + +For feature issues requiring documentation, follow the process below unless otherwise agreed with +the Product Manager and Technical Writer for a given issue: + +- Include any new and edited documentation, either in: + - The merge request introducing the code. + - A separate merge request raised around the same time. +- Use the [documentation requirements](#documentation-requirements) developed by the Product Manager + in the issue and discuss any further documentation plans or ideas as needed. + + If the new or changed documentation requires extensive collaboration or conversation, a + separate, linked issue can be used for the planning process. + +- Use the [Documentation guidelines](index.md), as well as other resources linked from there, + including: + - Documentation [Structure and template](structure.md) page. + - [Style Guide](styleguide.md). + - [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). +- Contact the Technical Writer for the relevant [DevOps stage](https://about.gitlab.com/handbook/product/technical-writing/index.html#assignments) + in your issue or merge request, or within `#docs` on GitLab Slack, if you: + - Need any help to choose the correct place for documentation. + - Want to discuss a documentation idea or outline. + - Want to request any other help. +- If you are working on documentation in a separate merge request, ensure the documentation is + merged as close as possible to the code merge. +- A policy for documenting [feature-flagged](../feature_flags/index.md) issues is forthcoming and you + are welcome to join the [discussion](https://gitlab.com/gitlab-org/gitlab/issues/26347). + +##### Reviews and merging + +Reviewers help ensure: + +- Accuracy. +- Clarity. +- Completeness. +- Adherence to: + - [Documentation requirements](#documentation-requirements) in the issue. + - [Documentation guidelines](index.md). + - [Style guide](styleguide.md). + +Prior to merging, documentation changes committed by the developer must be reviewed by: + +- The code reviewer for the merge request. This is known as a technical review. +- Optionally, others involved in the work, such as other developers or the Product Manager. +- Optionally, the Technical Writer for the DevOps stage group. +- A maintainer of the project. + +If not assigned to a Technical Writer for review prior to merging, a review must be scheduled +immediately after merge by the developer or maintainer. For this, +create an issue using the [Doc Review description template](https://gitlab.com/gitlab-org/gitlab/issues/new?issuable_template=Doc%20Review) +and link to it from the merged merge request that introduced the documentation change. + +To decide whether to request a Technical Writer review before or after merge, consider: + +- The amount of time left before the milestone release. If there is less than three days + remaining, seek a post-merge review and ping the writer via Slack to ensure the review is + completed in time. +- The size of the change and your degree of confidence in having early users (for example, + GitLab.com users) of features use your documentation as written. +- That pre-merge Technical Writer reviews should be most common when the code is complete well in + advance of a milestone release and for larger documentation changes. +- You can request a post-merge Technical Writer review if it's important to get the code part of + a merge request merged as soon as possible. +- The Technical Writer can also help decide that documentation can be merged without Technical + writer review, with the review to occur soon after merge. + +#### Product Managers + +Product Managers are responsible for the [documentation requirements](#documentation-requirements) +for a feature or feature enhancement. They can also: + +- Liaise with the Technical Writer for discussion and collaboration. +- Review documentation themselves. + +For issues requiring any new or updated documentation, the Product Manager must: + +- Add the ~documentation label. +- Confirm or add the [documentation requirements](#documentation-requirements). +- Ensure the issue contains: + - Any new or updated feature name. + - Overview, description, and use cases, as required by the + [documentation structure and template](structure.md), when applicable. + +Everyone is encouraged to draft the documentation requirements in the issue, but a Product Manager +will do the following: + +- When the issue is assigned a release milestone, review and update the Documentation details. +- By the kickoff, finalize the documentation details. + +#### Technical Writers + +Technical Writers are responsible for: + +- Reviewing documentation requirements in issues when called upon. +- Answering questions, and helping and providing advice throughout the authoring and editing + process. +- Reviewing all new and updated documentation content, whether before merge or after it is merged. +- Assisting the developer and Product Manager with feature documentation delivery. + +##### Planning + +The Technical Writer: + +- Reviews their group's `~feature` issues that are part of the next milestone to get a sense of the + scope of content likely to be authored. +- Recommends the `~documentation` label on issues from that list which don't have it but should, or + inquires with the PM to determine if documentation is truly required. +- For `~direction` issues from that list, reads the full issue and reviews its Documentation + requirements section. Addresses any recommendations or questions with the PMs and others + collaborating on the issue in order to refine or expand the Documentation requirements. + +##### Collaboration + +By default, the developer will work on documentation changes independently, but +the developer, Product Manager, or Technical Writer can propose a broader collaboration for +any given issue. + +Additionally, Technical Writers are available for questions at any time. + +##### Review + +Technical Writers: + +- Provide non-blocking reviews of all documentation changes, before or after the change is merged. +- Confirm that the documentation is: + - Clear. + - Grammatically correct. + - Discoverable. + - Navigable. +- Ensures that the documentation avoids: + - Redundancy. + - Bad file locations. + - Typos. + - Broken links. + +The Technical Writer will review the documentation to check that the developer and +code reviewer have ensured: + +- Clarity. +- Appropriate location, making sure the documentation is in the correct directories (often + reflecting how the product is structured) and has the correct name. +- Syntax, typos, and broken links. +- Improvements to the content. +- Accordance with the: + - [Documentation Style Guide](styleguide.md). + - [Structure and Template](structure.md) doc. + +### When documentation is required + +Documentation [is required](../contributing/merge_request_workflow.html#definition-of-done) for a +milestone when: + +- A new or enhanced feature is shipped that impacts the user of administrator experience. +- There are changes to the UI or API. +- A process, workflow, or previously documented feature is changed. +- A feature is deprecated or removed. + +NOTE: **Note:** +Documentation refactoring unrelated to a feature change is covered in the +[other process](#for-all-other-documentation), so that time-sensitive documentation updates are +prioritized. + +### Documentation requirements + +Requirements for the documentation of a feature should be included as part of the +issue for planning that feature in a **Documentation** section within the issue description. Issues +created using the [**Feature Proposal** template](https://gitlab.com/gitlab-org/gitlab/raw/master/.gitlab/issue_templates/Feature%20proposal.md) +have this section by default. + +Anyone can add these details, but the Product Manager who assigns the issue to a specific release +milestone will ensure these details are present and finalized by the time of that milestone's kickoff. + +Developers, Technical Writers, and others may help further refine this plan at any time. + +The following details should be included: + +- What concepts and procedures should the documentation guide and enable the user to understand or + accomplish? +- To this end, what new page(s) are needed, if any? What pages or subsections need updates? + Consider user, admin, and API documentation changes and additions. +- For any guide or instruction set, should it help address a single use case, or be flexible to + address a certain range of use cases? +- Do we need to update a previously recommended workflow? Should we link the new feature from + various relevant locations? Consider all ways documentation should be affected. +- Are there any key terms or task descriptions that should be included so that the documentation is + found in relevant searches? +- Include suggested titles of any pages or subsection headings, if applicable. +- List any documentation that should be cross-linked, if applicable. + +## For all other documentation + +These documentation changes are not associated with the release of a new or updated feature, and are +therefore labeled `backstage` in GitLab, rather than `feature`. They may include: + +- Documentation created or updated to improve accuracy, completeness, ease of use, or any reason + other than a [feature change](#for-a-product-change). +- Addressing gaps in existing documentation, or making improvements to existing documentation. +- Work on special projects related to the documentation. + +TIP: **Tip:** +Anyone can contribute a merge request or create an issue for GitLab's documentation. + +### Who updates the docs + +Anyone can contribute! You can create a merge request for documentation when: + +- You find errors or other room for improvement in existing documentation. +- You have an idea for all-new documentation that would help a GitLab user or administrator to + accomplish their work with GitLab. + +### How to update the docs + +To update GitLab documentation: + +1. Either: + - Click the **Edit this Page** link at the bottom of any page on <https://docs.gitlab.com>. + - Navigate to one of the repositories and documentation paths listed on the + [GitLab Documentation guidelines](index.md) page. +1. Follow the described standards and processes listed on the page, including: + - The [Structure and template](structure.md) page. + - The [Style Guide](styleguide.md). + - The [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/). +1. Follow GitLab's [Merge Request Guidelines](../contributing/merge_request_workflow.md#merge-request-guidelines). + +TIP: **Tip:** +Work in a fork if you do not have developer access to the GitLab project. + +Ping the Technical Writer for the relevant [DevOps stage group](https://about.gitlab.com/handbook/product/technical-writing/index.html#assignments) +in your issue or merge request, or within `#docs` if you are a member of GitLab's Slack workspace, if you: + +- Need help to choose the correct place for documentation. +- Want to discuss a documentation idea or outline. +- Want to request any other help. + +### Reviewing and merging + +Anyone with Maintainer access to the relevant GitLab project can merge documentation changes. +Maintainers must make a good-faith effort to ensure that the content: + +- Is clear and sufficiently easy for the intended audience to navigate and understand. +- Meets the [Documentation Guidelines](index.md) and [Style Guide](styleguide.md). + +If the author or reviewer has any questions, they can mention the writer who is assigned to the relevant +[DevOps stage group](https://about.gitlab.com/handbook/product/technical-writing/index.html#assignments). + +The process involves the following: + +- Primary Reviewer. Review by a [code reviewer](https://about.gitlab.com/handbook/engineering/projects/) + or other appropriate colleague to confirm accuracy, clarity, and completeness. This can be skipped + for minor fixes without substantive content changes. +- Technical Writer (Optional). If not completed for a merge request prior to merging, must be scheduled + post-merge. To request a: + - Pre-merge review, assign the Technical Writer listed for the applicable + [DevOps stage group](https://about.gitlab.com/handbook/product/technical-writing/index.html#assignments). + - Post-merge review, [create an issue for one](https://gitlab.com/gitlab-org/gitlab/issues/new?issuable_template=Doc%20Review) + and link it from the MR that makes the documentation change. +- Maintainer. For merge requests, Maintainers: + - Can always request any of the above reviews. + - Review before or after a Technical Writer review. + - Ensure the given release milestone is set. + - Ensure the appropriate labels are applied, including any required to pick a merge request into + a release. + - Ensure that, if there has not been a Technical Writer review completed or scheduled, they + [create the required issue](https://gitlab.com/gitlab-org/gitlab/issues/new?issuable_template=Doc%20Review), assign to the technical writer of the given stage group, + and link it from the merge request. + +The process is reflected in the **Documentation** +[merge request template](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/merge_request_templates/Documentation.md). + +### Other ways to help + +If you have ideas for further documentation resources please +[create an issue](https://gitlab.com/gitlab-org/gitlab/issues/new?issuable_template=Documentation) +using the Documentation template. diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index d34c3ce5ba1..cc9df479492 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -19,86 +19,22 @@ CE specs should remain untouched as much as possible and extra specs should be added for EE. Licensed features can be stubbed using the spec helper `stub_licensed_features` in `EE::LicenseHelpers`. -You can force Webpack to act as CE by either deleting the `ee/` directory or by -setting the [`IS_GITLAB_EE` environment variable](https://gitlab.com/gitlab-org/gitlab/blob/master/config/helpers/is_ee_env.js) -to something that evaluates as `false`. The same works for running tests -(for example `IS_GITLAB_EE=0 yarn jest`). +You can force GitLab to act as CE by either deleting the `ee/` directory or by +setting the [`FOSS_ONLY` environment variable](https://gitlab.com/gitlab-org/gitlab/blob/master/config/helpers/is_ee_env.js) +to something that evaluates as `true`. The same works for running tests +(for example `FOSS_ONLY=1 yarn jest`). [ee-as-ce]: https://gitlab.com/gitlab-org/gitlab/issues/2500 ## Separation of EE code -We want a [single code base][] eventually, but before we reach the goal, -we still need to merge changes from GitLab CE to EE. To help us get there, -we should make sure that we no longer edit CE files in place in order to -implement EE features. - -Instead, all EE code should be put inside the `ee/` top-level directory. The +All EE code should be put inside the `ee/` top-level directory. The rest of the code should be as close to the CE files as possible. -[single code base]: https://gitlab.com/gitlab-org/gitlab/issues/2952#note_41016454 - -### EE-specific comments - -When complete separation can't be achieved with the `ee/` directory, you can wrap -code in EE specific comments to designate the difference from CE/EE and add -some context for someone resolving a conflict. - -```rb -# EE-specific start -stub_licensed_features(variable_environment_scope: true) -# EE specific end -``` - -```haml --# EE-specific start -= render 'ci/variables/environment_scope', form_field: form_field, variable: variable --# EE-specific end -``` - -EE-specific comments should not be backported to CE. - -**Note:** This is only meant as a workaround, we should follow up and -resolve this soon. - -### Detection of EE-only files - -For each commit (except on `master`), the `ee-files-location-check` CI job tries -to detect if there are any new files that are EE-only. If any file is detected, -the job fails with an explanation of why and what to do to make it pass. - -Basically, the fix is simple: `git mv <file> ee/<file>`. - -#### How to name your branches? - -For any EE branch, the job will try to detect its CE counterpart by removing any -`ee-` prefix or `-ee` suffix from the EE branch name, and matching the last -branch that contains it. - -For instance, from the EE branch `new-shiny-feature-ee` (or -`ee-new-shiny-feature`), the job would find the corresponding CE branches: - -- `new-shiny-feature` -- `ce-new-shiny-feature` -- `new-shiny-feature-ce` -- `my-super-new-shiny-feature-in-ce` - -#### Whitelist some EE-only files that cannot be moved to `ee/` - -The `ee-files-location-check` CI job provides a whitelist of files or folders -that cannot or should not be moved to `ee/`. Feel free to open an issue to -discuss adding a new file/folder to this whitelist. - -For instance, it was decided that moving EE-only files from `qa/` to `ee/qa/` -would make it difficult to build the `gitLab-{ce,ee}-qa` Docker images and it -was [not worth the complexity]. - -[not worth the complexity]: https://gitlab.com/gitlab-org/gitlab/issues/4997#note_59764702 - ### EE-only features If the feature being developed is not present in any form in CE, we don't -need to put the codes under `EE` namespace. For example, an EE model could +need to put the code under the `EE` namespace. For example, an EE model could go into: `ee/app/models/awesome.rb` using `Awesome` as the class name. This is applied not only to models. Here's a list of other examples: @@ -116,7 +52,7 @@ is applied not only to models. Here's a list of other examples: - `ee/app/views/foo.html.haml` - `ee/app/views/foo/_bar.html.haml` -This works because for every path that are present in CE's eager-load/auto-load +This works because for every path that is present in CE's eager-load/auto-load paths, we add the same `ee/`-prepended path in [`config/application.rb`]. This also applies to views. @@ -170,7 +106,7 @@ still having access the class's implementation with `super`. There are a few gotchas with it: -- you should always [`extend ::Gitlab::Utils::Override`](utilities.md#overridehttpsgitlabcomgitlab-orggitlab-fossblobmasterlibgitlabutilsoverriderb) and use `override` to +- you should always [`extend ::Gitlab::Utils::Override`](utilities.md#override) and use `override` to guard the "overrider" method to ensure that if the method gets renamed in CE, the EE override won't be silently forgotten. - when the "overrider" would add a line in the middle of the CE @@ -441,13 +377,10 @@ CE and EE. The advantages of this: -- Minimal code difference between CE and EE. -- Very clear hints about where we're extending EE views while reading CE codes. +- Very clear hints about where we're extending EE views while reading CE code. The disadvantage of this: -- Slightly more work while developing EE features, because now we need to - port `render_if_exists` to CE. - If we have typos in the partial name, it would be silently ignored. ##### Caveats @@ -858,7 +791,7 @@ end ### Code in `spec/` When you're testing EE-only features, avoid adding examples to the -existing CE specs. Also do no change existing CE examples, since they +existing CE specs. Also do not change existing CE examples, since they should remain working as-is when EE is running without a license. Instead place EE specs in the `ee/spec` folder. @@ -992,10 +925,8 @@ For regular JS files, the approach is similar. ## SCSS code in `assets/stylesheets` -To separate EE-specific styles in SCSS files, if a component you're adding styles for -is limited to only EE, it is better to have a separate SCSS file in appropriate directory -within `app/assets/stylesheets`. -See [backporting changes](#backporting-changes-from-ee-to-ce) for instructions on how to merge changes safely. +If a component you're adding styles for is limited to EE, it is better to have a +separate SCSS file in an appropriate directory within `app/assets/stylesheets`. In some cases, this is not entirely possible or creating dedicated SCSS file is an overkill, e.g. a text style of some component is different for EE. In such cases, @@ -1037,24 +968,6 @@ to avoid conflicts during CE to EE merge. // EE-specific end ``` -## Backporting changes from EE to CE - -Until the work completed to merge the ce and ee codebases, which is tracked on [epic &802](https://gitlab.com/groups/gitlab-org/-/epics/802), there exists times in which some changes for EE require specific changes to the CE -code base. Examples of backports include the following: - -- Features intended or originally built for EE that are later decided to move to CE -- Sometimes some code in CE may impact the EE feature - -Here is a workflow to make sure those changes end up backported safely into CE too. - -1. **Make your changes in the EE branch.** If possible, keep a separated commit (to be squashed) to help backporting and review. -1. **Open merge request to EE project.** -1. **Apply the changes you made to CE files in a branch of the CE project.** (Tip: Use `patch` with the diff from your commit in EE branch) -1. **Open merge request to CE project**, referring it's a backport of EE changes and link to MR open in EE. -1. Once EE MR is merged, the MR towards CE can be merged. **But not before**. - -**Note:** regarding SCSS, make sure the files living outside `/ee/` don't diverge between CE and EE projects. - ## GitLab-svgs Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 1475e356b5b..1375bd6d56d 100644 --- a/doc/development/elasticsearch.md +++ b/doc/development/elasticsearch.md @@ -1,8 +1,9 @@ # Elasticsearch knowledge **(STARTER ONLY)** -This area is to maintain a compendium of useful information when working with elasticsearch. +This area is to maintain a compendium of useful information when working with Elasticsearch. -Information on how to enable Elasticsearch and perform the initial indexing is kept in ../integration/elasticsearch.md#enabling-elasticsearch +Information on how to enable Elasticsearch and perform the initial indexing is in +the [Elasticsearch integration documentation](../integration/elasticsearch.md#enabling-elasticsearch). ## Deep Dive @@ -59,9 +60,9 @@ Additionally, if you need large repos or multiple forks for testing, please cons ## How does it work? -The Elasticsearch integration depends on an external indexer. We ship an [indexer written in Go](https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer). The user must trigger the initial indexing via a rake task but, after this is done, GitLab itself will trigger reindexing when required via `after_` callbacks on create, update, and destroy that are inherited from [/ee/app/models/concerns/elastic/application_search.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/elastic/application_search.rb). +The Elasticsearch integration depends on an external indexer. We ship an [indexer written in Go](https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer). The user must trigger the initial indexing via a rake task but, after this is done, GitLab itself will trigger reindexing when required via `after_` callbacks on create, update, and destroy that are inherited from [/ee/app/models/concerns/elastic/application_versioned_search.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/elastic/application_versioned_search.rb). -All indexing after the initial one is done via `ElasticIndexerWorker` (sidekiq jobs). +All indexing after the initial one is done via `ElasticIndexerWorker` (Sidekiq jobs). Search queries are generated by the concerns found in [ee/app/models/concerns/elastic](https://gitlab.com/gitlab-org/gitlab/tree/master/ee/app/models/concerns/elastic). These concerns are also in charge of access control, and have been a historic source of security bugs so please pay close attention to them! @@ -243,4 +244,4 @@ cluster.routing.allocation.disk.watermark.high: 10gb Restart Elasticsearch, and the `read_only_allow_delete` will clear on it's own. -_from "Disk-based Shard Allocation | Elasticsearch Reference" [5.6](https://www.elastic.co/guide/en/elasticsearch/reference/5.6/disk-allocator.html#disk-allocator) and [6.x](https://www.elastic.co/guide/en/elasticsearch/reference/6.x/disk-allocator.html)_ +_from "Disk-based Shard Allocation | Elasticsearch Reference" [5.6](https://www.elastic.co/guide/en/elasticsearch/reference/5.6/disk-allocator.html#disk-allocator) and [6.x](https://www.elastic.co/guide/en/elasticsearch/reference/6.7/disk-allocator.html)_ diff --git a/doc/development/emails.md b/doc/development/emails.md index 91b9e11f7f6..2c5f3be45d8 100644 --- a/doc/development/emails.md +++ b/doc/development/emails.md @@ -17,10 +17,9 @@ dummy data. The previews live in [`app/mailers/previews`][previews] and can be viewed at [`/rails/mailers`](http://localhost:3000/rails/mailers). -See the [Rails guides] for more info. +See the [Rails guides](https://guides.rubyonrails.org/action_mailer_basics.html#previewing-emails) for more info. [previews]: https://gitlab.com/gitlab-org/gitlab-foss/tree/master/app/mailers/previews -[Rails guides]: http://guides.rubyonrails.org/action_mailer_basics.html#previewing-emails ## Incoming email @@ -88,15 +87,15 @@ for the format of the email key: - Actions are always at the end, separated by `-`. For example `-issue` or `-merge-request` - If your feature is related to a project, the key begins with the project identifiers (project path slug - and project id), separated by `-`. For example, `gitlab-org-gitlab-ce-20` + and project id), separated by `-`. For example, `gitlab-org-gitlab-foss-20` - Additional information, such as an author's token, can be added between the project identifiers and - the action, separated by `-`. For example, `gitlab-org-gitlab-ce-20-Author_Token12345678-issue` + the action, separated by `-`. For example, `gitlab-org-gitlab-foss-20-Author_Token12345678-issue` - You register your handlers in `lib/gitlab/email/handler.rb` Examples of valid email keys: -- `gitlab-org-gitlab-ce-20-Author_Token12345678-issue` (create a new issue) -- `gitlab-org-gitlab-ce-20-Author_Token12345678-merge-request` (create a new merge request) +- `gitlab-org-gitlab-foss-20-Author_Token12345678-issue` (create a new issue) +- `gitlab-org-gitlab-foss-20-Author_Token12345678-merge-request` (create a new merge request) - `1234567890abcdef1234567890abcdef-unsubscribe` (unsubscribe from a conversation) - `1234567890abcdef1234567890abcdef` (reply to a conversation) diff --git a/doc/development/event_tracking/frontend.md b/doc/development/event_tracking/frontend.md index cdc2e94bf9e..c767efc65b2 100644 --- a/doc/development/event_tracking/frontend.md +++ b/doc/development/event_tracking/frontend.md @@ -1,6 +1,6 @@ # Frontend tracking guide -GitLab provides `Tracking`, an interface that wraps the [Snowplow Javascript Tracker](https://github.com/snowplow/snowplow/wiki/javascript-tracker) for tracking custom events. There are a few ways to utilizing tracking, but each generally requires at minimum, a `category` and an `action`. Additional data can be provided that adheres to our [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). +GitLab provides `Tracking`, an interface that wraps the [Snowplow JavaScript Tracker](https://github.com/snowplow/snowplow/wiki/javascript-tracker) for tracking custom events. There are a few ways to utilize tracking, but each generally requires at minimum, a `category` and an `action`. Additional data can be provided that adheres to our [Feature instrumentation taxonomy](https://about.gitlab.com/handbook/product/feature-instrumentation/#taxonomy). | field | type | default value | description | |:-----------|:-------|:---------------------------|:------------| @@ -26,7 +26,7 @@ Below is an example of `data-track-*` attributes assigned to a button: /> ``` -Event listeners are bound at the document level to handle click events on or within elements with these data attributes. This allows for them to be properly handled on rerendering and changes to the DOM, but it's important to know that because of the way these events are bound, click events shouldn't be stopped from propagating up the DOM tree. If for any reason click events are being stopped from propagating, you'll need to implement your own listeners and follow the instructions in [Tracking in raw Javascript](#tracking-in-raw-javascript). +Event listeners are bound at the document level to handle click events on or within elements with these data attributes. This allows for them to be properly handled on rerendering and changes to the DOM, but it's important to know that because of the way these events are bound, click events shouldn't be stopped from propagating up the DOM tree. If for any reason click events are being stopped from propagating, you'll need to implement your own listeners and follow the instructions in [Tracking in raw JavaScript](#tracking-in-raw-javascript). Below is a list of supported `data-track-*` attributes: @@ -99,7 +99,7 @@ And if needed within the template, you can use the `track` method directly as we </template> ``` -## Tracking in raw Javascript +## Tracking in raw JavaScript Custom event tracking and instrumentation can be added by directly calling the `Tracking.event` static function. The following example demonstrates tracking a click on a button by calling `Tracking.event` manually. diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md new file mode 100644 index 00000000000..5155433c9ad --- /dev/null +++ b/doc/development/experiment_guide/index.md @@ -0,0 +1,65 @@ +# Experiment Guide + +Experiments will be conducted by teams from the [Growth Section](https://about.gitlab.com/handbook/engineering/development/growth/) and are not tied to releases, because they will primarily target GitLab.com. + +Experiments will be run as an A/B test and will be behind a feature flag to turn the test on or off. Based on the data the experiment generates, the team decides if the experiment had a positive impact and will be the new default or rolled back. + +## Follow-up issue + +Each experiment requires a follow-up issue to resolve the experiment. Immediately after an experiment is deployed, the deadline of the issue needs to be set (this depends on the experiment but can be up to a few weeks in the future). +After the deadline, the issue needs to be resolved and either: + +- It was successful and the experiment will be the new default. +- It was not successful and all code related to the experiment will be removed. + +In either case, an outcome of the experiment should be posted to the issue with the reasoning for the decision. + +## Code reviews + +Since the code of experiments will not be part of the codebase for a long time and we want to iterate fast to retrieve data,the code quality of experiments might sometimes not fulfill our standards but should not negatively impact the availability of GitLab whether the experiment is running or not. +Experiments will only be deployed to a fraction of users but we still want a flawless experience for those users. Therefore, experiments still require tests. + +For reviewers and maintainers: if you find code that would usually not make it through the review, but is temporarily acceptable, please mention your concerns but note that it's not necessary to change. +The author then adds a comment to this piece of code and adds a link to the issue that resolves the experiment. + +## How to create an A/B test + +- [ ] Add the experiment to the `Gitlab::Experimentation::EXPERIMENTS` hash in [`experimentation.rb`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib%2Fgitlab%2Fexperimentation.rb): + + ```ruby + EXPERIMENTS = { + other_experiment: { + #... + }, + # Add your experiment here: + sign_up_flow: { + feature_toggle: :experimental_sign_up_flow, # Feature flag that will be used + environment: ::Gitlab.dev_env_or_com?, # Target environment + enabled_ratio: 0.1 # Percentage of users that will be part of the experiment. 10% of the users would be part of this experiments. + } + }.freeze + ``` + +- [ ] Use the experiment in a controller: + + ```ruby + class RegistrationController < Applicationcontroller + def show + # experiment_enabled?(:feature_name) is also available in views and helpers + if experiment_enabled?(:sign_up_flow) + # render the experiment + else + # render the original version + end + end + end + ``` + +- [ ] Track necessery events. See the [event tracking guide](../event_tracking/index.md) for details. +- [ ] After the merge request is merged, use [`chatops`](../../ci/chatops/README.md) to enable the feature flag and start the experiment. For visibility, please run the command in the `#s_growth` channel: + + ``` + /chatops run feature set --project=gitlab-org/gitlab experimental_sign_up_flow true + ``` + + If you notice issues with the experiment, you can disable the experiment by setting the feature flag to `false` again. diff --git a/doc/development/fe_guide/components.md b/doc/development/fe_guide/components.md index e88827f78c1..6b6274a6480 100644 --- a/doc/development/fe_guide/components.md +++ b/doc/development/fe_guide/components.md @@ -1,63 +1,3 @@ -# Components - -## Contents - -- [Dropdowns](#dropdowns) -- [Modals](#modals) - -## Dropdowns - -See also the [corresponding UX guide](https://design.gitlab.com/#/components/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! - ``` - -[bootstrap-dropdowns]: https://getbootstrap.com/docs/3.3/javascript/#dropdowns - -## Modals - -See also the [corresponding UX guide](https://design.gitlab.com/#/components/modals). - -We have a reusable Vue component for modals: [vue_shared/components/gl_modal.vue](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/javascripts/vue_shared/components/gl_modal.vue) - -Here is an example of how to use it: - -```html - <gl-modal - id="dogs-out-modal" - :header-title-text="s__('ModalExample|Let the dogs out?')" - footer-primary-button-variant="danger" - :footer-primary-button-text="s__('ModalExample|Let them out')" - @submit="letOut(theDogs)" - > - {{ s__('ModalExample|You’re about to let the dogs out.') }} - </gl-modal> -``` - - +--- +redirect_to: 'https://design.gitlab.com/components/status/' +--- diff --git a/doc/development/fe_guide/design_patterns.md b/doc/development/fe_guide/design_patterns.md index 0893299540f..a7a0f39e2f3 100644 --- a/doc/development/fe_guide/design_patterns.md +++ b/doc/development/fe_guide/design_patterns.md @@ -77,4 +77,4 @@ new Foo({ container: '.my-element' }); You can find an example of the above in this [class][container-class-example]; -[container-class-example]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js +[container-class-example]: https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index 41513e6d57e..3724bf60757 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -80,7 +80,7 @@ With the purpose of being [respectful of others' time](https://about.gitlab.com/ 1. Before writing code, ensure your vision of the architecture is aligned with GitLab's architecture. -1. Add a diagram to the issue and ask a frontend architect in the slack channel `#fe_architectural` about it. +1. Add a diagram to the issue and ask a frontend maintainer in the Slack channel `#frontend_maintainers` about it.  diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md index e9d6244355c..019bd36573d 100644 --- a/doc/development/fe_guide/dropdowns.md +++ b/doc/development/fe_guide/dropdowns.md @@ -1 +1,3 @@ -This page has moved [here](components.md#dropdowns). +--- +redirect_to: 'https://design.gitlab.com/components/dropdowns/' +--- diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 2ec3f8017a1..36c8a03a241 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -26,7 +26,7 @@ question: document.body.dataset.page ``` -Find here the [source code setting the attribute](https://gitlab.com/gitlab-org/gitlab-foss/blob/cc5095edfce2b4d4083a4fb1cdc7c0a1898b9921/app/views/layouts/application.html.haml#L4). +Find here the [source code setting the attribute](https://gitlab.com/gitlab-org/gitlab/blob/cc5095edfce2b4d4083a4fb1cdc7c0a1898b9921/app/views/layouts/application.html.haml#L4). #### Rails routes diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 366c2894b81..fe4f6d7bec8 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -1,11 +1,18 @@ # GraphQL +Our GraphQL API can be explored via GraphiQL at your instance's +`/-/graphql-explorer` or at [GitLab.com](https://gitlab.com/-/graphql-explorer). + +You can check all existing queries and mutations on the right side +of GraphiQL in its **Documentation explorer**. It's also possible to +write queries and mutations directly on the left tab and check +their execution by clicking **Execute query** button on the top left: + + + We use [Apollo] and [Vue Apollo][vue-apollo] for working with GraphQL on the frontend. -In order to use GraphQL, you need to enable the `graphql` feature flag, -read more about [Feature Flags][feature-flags]. - ## Apollo Client To save duplicated clients getting created in different apps, we have a @@ -119,6 +126,6 @@ Read more about the [Apollo] client in the [Apollo documentation](https://www.ap [Apollo]: https://www.apollographql.com/ [vue-apollo]: https://github.com/Akryum/vue-apollo/ [feature-flags]: ../feature_flags.md -[default-client]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/javascripts/lib/graphql.js +[default-client]: https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/javascripts/lib/graphql.js [vue-test-utils]: https://vue-test-utils.vuejs.org/ [apollo-link-state]: https://www.apollographql.com/docs/link/links/state.html diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index 4f687d8642e..d81a520c5f3 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -1,6 +1,6 @@ # Icons and SVG Illustrations -We manage our own Icon and Illustration library in the [gitlab-svgs][gitlab-svgs] repository. +We manage our own Icon and Illustration library in the [`gitlab-svgs`][gitlab-svgs] repository. This repository is published on [npm][npm] and managed as a dependency via yarn. You can browse all available Icons and Illustrations [here][svg-preview]. To upgrade to a new version run `yarn upgrade @gitlab/svgs`. @@ -59,8 +59,8 @@ export default { <template> <icon name="issues" - :size="72" - css-classes="icon-danger" + :size="24" + class="icon-danger" /> </template> ``` diff --git a/doc/development/fe_guide/img/graphiql_explorer_v12_4.png b/doc/development/fe_guide/img/graphiql_explorer_v12_4.png Binary files differnew file mode 100644 index 00000000000..8981b37ba23 --- /dev/null +++ b/doc/development/fe_guide/img/graphiql_explorer_v12_4.png diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 4cccd4aa5fb..1cf798cedb6 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -5,8 +5,8 @@ across GitLab's frontend team. ## Overview -GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] and also a JavaScript based Frontend with [Vue.js][vue]. -Be wary of [the limitations that come with using Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org) using [Haml][haml] and also a JavaScript based Frontend with [Vue.js](https://vuejs.org). +Be wary of [the limitations that come with using Hamlit][hamlit-limits]. We also use [SCSS](https://sass-lang.com) and plain JavaScript with modern ECMAScript standards supported through [Babel][babel] and ES module support through [webpack][webpack]. Working with our frontend assets requires Node (v8.10.0 or greater) and Yarn @@ -41,6 +41,11 @@ or make changes to our frontend development guidelines. How we write frontend tests, run the GitLab test suite, and debug test related issues. +## Pajamas Design System + +Reusable components with technical and usage guidelines can be found in our +[Pajamas Design System](https://design.gitlab.com/). + ## [Design Patterns](design_patterns.md) Common JavaScript design patterns in GitLab's codebase. @@ -65,10 +70,6 @@ How to use GraphQL How we use SVG for our Icons and Illustrations. -## [Components](components.md) - -How we use UI components. - ## Frontend FAQ Read the [frontend's FAQ](frontend_faq.md) for common small pieces of helpful information. @@ -83,7 +84,7 @@ changes. ### [SCSS Style Guide](style_guide_scss.md) -Our SCSS conventions which are enforced through [scss-lint][scss-lint]. +Our SCSS conventions which are enforced through [scss-lint](https://github.com/sds/scss-lint). ## [Performance](performance.md) @@ -102,17 +103,13 @@ Our accessibility standards and resources. Frontend internationalization support is described in [this document](../i18n/). The [externalization part of the guide](../i18n/externalization.md) explains the helpers/methods available. -[rails]: http://rubyonrails.org/ [haml]: http://haml.info/ [hamlit]: https://github.com/k0kubun/hamlit [hamlit-limits]: https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations -[scss]: http://sass-lang.com/ [babel]: https://babeljs.io/ [webpack]: https://webpack.js.org/ [jquery]: https://jquery.com/ -[vue]: http://vuejs.org/ [axios]: https://github.com/axios/axios [airbnb-js-style-guide]: https://github.com/airbnb/javascript -[scss-lint]: https://github.com/brigade/scss-lint [install]: ../../install/installation.md#4-node [requirements]: ../../install/requirements.md#supported-web-browsers diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index bcd22a170e1..6d5021c0f08 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -65,26 +65,27 @@ within the `pages` directory correspond to Rails controllers and actions. These auto-generated bundles will be automatically included on the corresponding pages. -For example, if you were to visit [gitlab.com/gitlab-org/gitlab-foss/issues](https://gitlab.com/gitlab-org/gitlab-foss/issues), +For example, if you were to visit <https://gitlab.com/gitlab-org/gitlab/issues>, you would be accessing the `app/controllers/projects/issues_controller.rb` controller with the `index` action. If a corresponding file exists at `pages/projects/issues/index/index.js`, it will be compiled into a webpack bundle and included on the page. -> **Note:** Previously we had encouraged the use of -> `content_for :page_specific_javascripts` within haml files, along with -> manually generated webpack bundles. However under this new system you should -> not ever need to manually add an entry point to the `webpack.config.js` file. -> -> **Tip:** -> If you are unsure what controller and action corresponds to a given page, you -> can find this out by inspecting `document.body.dataset.page` within your -> browser's developer console while on any page within gitlab. +NOTE: **Note:** +Previously we had encouraged the use of +`content_for :page_specific_javascripts` within haml files, along with +manually generated webpack bundles. However under this new system you should +not ever need to manually add an entry point to the `webpack.config.js` file. + +TIP: **Tip:** +If you are unsure what controller and action corresponds to a given page, you +can find this out by inspecting `document.body.dataset.page` within your +browser's developer console while on any page within GitLab. #### Important Considerations - **Keep Entry Points Lite:** - Page-specific javascript entry points should be as lite as possible. These + Page-specific JavaScript entry points should be as lite as possible. These files are exempt from unit tests, and should be used primarily for instantiation and dependency injection of classes and methods that live in modules outside of the entry point script. Just import, read the DOM, @@ -165,14 +166,12 @@ General tips: ## Additional Resources -- [WebPage Test][web-page-test] for testing site loading time and size. +- [WebPage Test](https://www.webpagetest.org) for testing site loading time and size. - [Google PageSpeed Insights][pagespeed-insights] grades web pages and provides feedback to improve the page. -- [Profiling with Chrome DevTools][google-devtools-profiling] +- [Profiling with Chrome DevTools](https://developers.google.com/web/tools/chrome-devtools/) - [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance. -[web-page-test]: http://www.webpagetest.org/ [pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/ -[google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en [browser-diet]: https://browserdiet.com/ [high-perf-animations]: https://www.html5rocks.com/en/tutorials/speed/high-performance-animations/ [flip]: https://aerotwist.com/blog/flip-your-animations/ diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md index 47ac87fc895..7dba61d6b45 100644 --- a/doc/development/fe_guide/security.md +++ b/doc/development/fe_guide/security.md @@ -63,7 +63,7 @@ External fonts, CSS, and JavaScript should never be used with the exception of Google Analytics and Piwik - and only when the instance has enabled it. Assets should always be hosted and served locally from the GitLab instance. Embedded resources via `iframes` should never be used except in certain circumstances -such as with ReCaptcha, which cannot be used without an `iframe`. +such as with reCAPTCHA, which cannot be used without an `iframe`. ## Avoiding inline scripts and styles diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index db076243812..306b19c6e5d 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -7,7 +7,7 @@ See the relevant style guides for our guidelines and for information on linting: We defer to [Airbnb][airbnb-js-style-guide] on most style-related conventions and enforce them with eslint. -See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/.eslintrc.yml) for specific rules and patterns. +See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab/blob/master/.eslintrc.yml) for specific rules and patterns. ### Common @@ -287,7 +287,7 @@ See [our current .eslintrc](https://gitlab.com/gitlab-org/gitlab-foss/blob/maste #### CSS classes used for JavaScript -1. If the class is being used in Javascript it needs to be prepend with `js-` +1. If the class is being used in JavaScript it needs to be prepend with `js-` ```html // bad @@ -693,7 +693,7 @@ Useful links: $('span').tooltip('_fixTitle'); ``` -### The Javascript/Vue Accord +### The JavaScript/Vue Accord The goal of this accord is to make sure we are all on the same page. @@ -713,7 +713,7 @@ The goal of this accord is to make sure we are all on the same page. - [SCSS](style_guide_scss.md) [airbnb-js-style-guide]: https://github.com/airbnb/javascript -[eslintrc]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/.eslintrc +[eslintrc]: https://gitlab.com/gitlab-org/gitlab/blob/master/.eslintrc [eslint-plugin-vue]: https://github.com/vuejs/eslint-plugin-vue [eslint-plugin-vue-rules]: https://github.com/vuejs/eslint-plugin-vue#bulb-rules [vue-order]: https://github.com/vuejs/eslint-plugin-vue/blob/master/docs/rules/order-in-components.md diff --git a/doc/development/fe_guide/style_guide_scss.md b/doc/development/fe_guide/style_guide_scss.md index d5af19e0ea4..07c87920dab 100644 --- a/doc/development/fe_guide/style_guide_scss.md +++ b/doc/development/fe_guide/style_guide_scss.md @@ -13,12 +13,12 @@ led by the [GitLab UI WG](https://gitlab.com/gitlab-com/www-gitlab-com/merge_req #### Where are utility classes defined? - [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/) -- [`common.scss`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/stylesheets/framework/common.scss) (old) -- [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/stylesheets/utilities.scss) (new) +- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old) +- [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss) (new) #### Where should I put new utility classes? -New utility classes should be added to [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/stylesheets/utilities.scss). Existing classes include: +New utility classes should be added to [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss). Existing classes include: | Name | Pattern | Example | |------|---------|---------| @@ -27,8 +27,8 @@ New utility classes should be added to [`utilities.scss`](https://gitlab.com/git | Font size | `.text-{size}` | `.text-2` | - `{variant}` is one of 'primary', 'secondary', 'success', 'warning', 'error' -- `{shade}` is on of the shades listed on [colors](https://design.gitlab.com/foundations/colors/) -- `{size}` is a number from 1-6 from our [Type scale](https://design.gitlab.com/foundations/typography) +- `{shade}` is on of the shades listed on [colors](https://design.gitlab.com/product-foundations/colors/) +- `{size}` is a number from 1-6 from our [Type scale](https://design.gitlab.com/product-foundations/typography) #### When should I create component classes? @@ -41,13 +41,13 @@ This encourages an organic growth of component classes and prevents the creation Examples of component classes that were created using "utility-first" include: -- [`.circle-icon-container`](https://gitlab.com/gitlab-org/gitlab-foss/blob/579fa8b8ec7eb38d40c96521f517c9dab8c3b97a/app/assets/stylesheets/framework/icons.scss#L85) -- [`.d-flex-center`](https://gitlab.com/gitlab-org/gitlab-foss/blob/900083d89cd6af391d26ab7922b3f64fa2839bef/app/assets/stylesheets/framework/common.scss#L425) +- [`.circle-icon-container`](https://gitlab.com/gitlab-org/gitlab/blob/579fa8b8ec7eb38d40c96521f517c9dab8c3b97a/app/assets/stylesheets/framework/icons.scss#L85) +- [`.d-flex-center`](https://gitlab.com/gitlab-org/gitlab/blob/900083d89cd6af391d26ab7922b3f64fa2839bef/app/assets/stylesheets/framework/common.scss#L425) Inspiration: -- <https://tailwindcss.com/docs/utility-first> -- <https://tailwindcss.com/docs/extracting-components> +- <https://tailwindcss.com/docs/utility-first/> +- <https://tailwindcss.com/docs/extracting-components/> ### Naming @@ -236,7 +236,7 @@ Before adding a new variable for a color or a size, guarantee: ## Linting -We use [SCSS Lint][scss-lint] to check for style guide conformity. It uses the +We use [SCSS Lint](https://github.com/sds/scss-lint) to check for style guide conformity. It uses the ruleset in `.scss-lint.yml`, which is located in the home directory of the project. @@ -245,7 +245,7 @@ scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI to catch any warnings. If the Rake task is throwing warnings you don't understand, SCSS Lint's -documentation includes [a full list of their linters][scss-lint-documentation]. +documentation includes [a full list of their linters][scss-lint-documentation](https://github.com/sds/scss-lint/blob/master/lib/scss_lint/linter/README.md). ### Fixing issues @@ -260,7 +260,7 @@ Note that this won't fix every problem, but it should fix a majority. ### Ignoring issues If you want a line or set of lines to be ignored by the linter, you can use -`// scss-lint:disable RuleName` ([more info][disabling-linters]): +`// scss-lint:disable RuleName` ([more info](https://github.com/sds/scss-lint#disabling-linters-via-source)): ```scss // This lint rule is disabled because it is supported only in Chrome/Safari @@ -279,6 +279,3 @@ guide is ignored in this instance. [csscomb]: https://github.com/csscomb/csscomb.js [node]: https://github.com/nodejs/node [npm]: https://www.npmjs.com/ -[scss-lint]: https://github.com/brigade/scss-lint -[scss-lint-documentation]: https://github.com/brigade/scss-lint/blob/master/lib/scss_lint/linter/README.md -[disabling-linters]: https://github.com/brigade/scss-lint#disabling-linters-via-source diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 396467b47d1..bca24e6ee0b 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -1,6 +1,6 @@ # Vue -To get started with Vue, read through [their documentation][vue-docs]. +To get started with Vue, read through [their documentation](https://vuejs.org/v2/guide/). ## Examples @@ -104,6 +104,51 @@ document.addEventListener('DOMContentLoaded', () => new Vue({ })); ``` +#### Accessing feature flags + +Use Vue's [provide/inject](https://vuejs.org/v2/api/#provide-inject) mechanism +to make feature flags available to any descendant components in a Vue +application. The `glFeatures` object is already provided in `commons/vue.js`, so +only the mixin is required to utilize the flags: + +```javascript +// An arbitrary descendant component + +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + +export default { + // ... + mixins: [glFeatureFlagsMixin()], + // ... + created() { + if (this.glFeatures.myFlag) { + // ... + } + }, +} +``` + +This approach has a few benefits: + +- Arbitrarily deeply nested components can opt-in and access the flag without + intermediate components being aware of it (c.f. passing the flag down via + props). +- Good testability, since the flag can be provided to `mount`/`shallowMount` + from `vue-test-utils` as easily as a prop. + + ```javascript + import { shallowMount } from '@vue/test-utils'; + + shallowMount(component, { + provide: { + glFeatures: { myFlag: true }, + }, + }); + ``` + +- No need to access a global variable, except in the application's + [entry point](#accessing-the-gl-object). + ### A folder for Components This folder holds all components that are specific of this new feature. @@ -245,7 +290,6 @@ One should apply to be a Vue.js expert by opening an MR when the Merge Request's - Vuex code follows the [documented pattern](vuex.md#actions-pattern-request-and-receive-namespaces) - Knowledge about the existing Vue and Vuex applications and existing reusable components -[vue-docs]: http://vuejs.org/guide/index.html [issue-boards]: https://gitlab.com/gitlab-org/gitlab-foss/tree/master/app/assets/javascripts/boards [environments-table]: https://gitlab.com/gitlab-org/gitlab-foss/tree/master/app/assets/javascripts/environments [page_specific_javascript]: ./performance.md#page-specific-javascript @@ -253,5 +297,5 @@ One should apply to be a Vue.js expert by opening an MR when the Merge Request's [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-test]: https://vuejs.org/v2/guide/unit-testing.html -[flux]: https://facebook.github.io/flux +[flux]: https://facebook.github.io/flux/ [axios]: https://github.com/axios/axios diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index bb131746ecf..20abf4fcb3e 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -232,7 +232,7 @@ import { mapGetters } from 'vuex'; ### `mutation_types.js` -From [vuex mutations docs][vuex-mutations]: +From [vuex mutations docs](https://vuex.vuejs.org/guide/mutations.html): > 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. ```javascript @@ -336,7 +336,7 @@ export default { #### Testing Vuex concerns -Refer to [vuex docs][vuex-testing] regarding testing Actions, Getters and Mutations. +Refer to [vuex docs](https://vuex.vuejs.org/guide/testing.html) regarding testing Actions, Getters and Mutations. #### Testing components that need a store @@ -396,6 +396,3 @@ export default () => {}; ``` [vuex-docs]: https://vuex.vuejs.org -[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 diff --git a/doc/development/feature_flags/controls.md b/doc/development/feature_flags/controls.md index 739f4207e27..f22c3bb1e37 100644 --- a/doc/development/feature_flags/controls.md +++ b/doc/development/feature_flags/controls.md @@ -1,9 +1,11 @@ -# Access for enabling a feature flag in production +# Feature flag controls -In order to be able to turn on/off features behind feature flags in any of the +## Access + +To be able to turn on/off features behind feature flags in any of the GitLab Inc. provided environments such as staging and production, you need to -have access to the chatops bot. Chatops bot is currently running on the ops instance, -which is different from GitLab.com or dev.gitlab.org. +have access to the [Chatops](../chatops_on_gitlabcom.md) bot. The Chatops bot +is currently running on the ops instance, which is different from <https://gitlab.com> or <https://dev.gitlab.org>. Follow the Chatops document to [request access](../chatops_on_gitlabcom.md#requesting-access). @@ -14,6 +16,19 @@ run: /chatops run feature --help ``` +## Where to run commands + +To increase visibility, we recommend that GitLab team members run feature flag +related Chatops commands within certain slack channels based on the environment +and related feature. For the [staging](https://staging.gitlab.com) +and [development](https://dev.gitlab.org) environments of GitLab.com, +the commands should run in a channel for the stage the feature is relevant too. + +For example, use the `#s_monitor` channel for features developed by the +Monitor stage, Health group. + +For all production environment Chatops commands, use the `#production` channel. + ## Rolling out changes When the changes are deployed to the environments it is time to start @@ -28,7 +43,7 @@ easier to measure the impact of both separately. GitLab's feature library (using [Flipper](https://github.com/jnunemaker/flipper), and covered in the [Feature Flags process](process.md) guide) supports rolling out changes to a percentage of -users. This in turn can be controlled using [GitLab chatops](../../ci/chatops/README.md). +users. This in turn can be controlled using [GitLab Chatops](../../ci/chatops/README.md). For an up to date list of feature flag commands please see [the source code](https://gitlab.com/gitlab-com/chatops/blob/master/lib/chatops/commands/feature.rb). @@ -37,9 +52,9 @@ Note that all the examples in that file must be preceded by If you get an error "Whoops! This action is not allowed. This incident will be reported." that means your Slack account is not allowed to -change feature flags or you do not [have access](#access-for-enabling-a-feature-flag-in-production). +change feature flags or you do not [have access](#access). -### Enabling feature for staging and dev.gitlab.org +### Enabling feature for preproduction testing As a first step in a feature rollout, you should enable the feature on <https://staging.gitlab.com> and <https://dev.gitlab.org>. @@ -64,7 +79,7 @@ there for any exceptions while testing your feature after enabling the feature f Once you are confident enough that these environments are in a good state with your feature enabled, you can roll out the change to GitLab.com. -## Enabling feature for GitLab.com +### Enabling a feature for GitLab.com Similar to above, to enable a feature for 25% of all users, run the following in Slack: @@ -91,11 +106,11 @@ sure it is clearly communicated to your team, and the Production team if you anticipate any potential problems. Feature gates can also be actor based, for example a feature could first be -enabled for only the `gitlab-ce` project. The project is passed by supplying a +enabled for only the `gitlab` project. The project is passed by supplying a `--project` flag: ``` -/chatops run feature set --project=gitlab-org/gitlab-ce some_feature true +/chatops run feature set --project=gitlab-org/gitlab some_feature true ``` For groups the `--group` flag is available: @@ -114,10 +129,17 @@ merge request has to be picked into a stable branch, make sure to also add the appropriate "Pick into X" label (e.g. "Pick into XX.X"). See [the process document](process.md#including-a-feature-behind-feature-flag-in-the-final-release) for further details. -When a feature gate has been removed from the code base, the value still exists -in the database. -This can be removed through ChatOps: +When a feature gate has been removed from the code base, the feature +record still exists in the database that the flag was deployed too. +The record can be deleted once the MR is deployed to each environment: +```sh +/chatops run feature delete some_feature --dev +/chatops run feature delete some_feature --staging ``` + +Then, you can delete it from production after the MR is deployed to prod: + +```sh /chatops run feature delete some_feature ``` diff --git a/doc/development/feature_flags/development.md b/doc/development/feature_flags/development.md index b338a191f76..929c9b1c71c 100644 --- a/doc/development/feature_flags/development.md +++ b/doc/development/feature_flags/development.md @@ -109,6 +109,9 @@ if ( gon.features.vimBindings ) { The name of the feature flag in JavaScript will always be camelCased, meaning that checking for `gon.features.vim_bindings` would not work. +See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about +how to access feature flags in a Vue component. + ### Specs In the test environment `Feature.enabled?` is stubbed to always respond to `true`, diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md index 8d486df9a8a..47a6babe8bc 100644 --- a/doc/development/file_storage.md +++ b/doc/development/file_storage.md @@ -146,4 +146,4 @@ end [CarrierWave]: https://github.com/carrierwaveuploader/carrierwave [Hashed Storage]: ../administration/repository_storage_types.md [all-in-one rake task]: ../administration/raketasks/uploads/migrate.md -[category list]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/tasks/gitlab/uploads/migrate.rake +[category list]: https://gitlab.com/gitlab-org/gitlab/blob/master/lib/tasks/gitlab/uploads/migrate.rake diff --git a/doc/development/filtering_by_label.md b/doc/development/filtering_by_label.md index 6aa7e7a5293..32df54eafd3 100644 --- a/doc/development/filtering_by_label.md +++ b/doc/development/filtering_by_label.md @@ -79,7 +79,7 @@ it did not improve query performance. ## Attempt B: Denormalize using an array column -Having [removed MySQL support in GitLab 12.1](https://about.gitlab.com/2019/06/27/removing-mysql-support/), +Having [removed MySQL support in GitLab 12.1](https://about.gitlab.com/blog/2019/06/27/removing-mysql-support/), using [Postgres's arrays](https://www.postgresql.org/docs/9.6/arrays.html) became more tractable as we didn't have to support two databases. We discussed denormalizing the `label_links` table for querying in diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index 792ddeed201..64f283f69d9 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -15,9 +15,9 @@ In May 2019, Bob Van Landuyt hosted a [Deep Dive] on GitLab's [Gitaly project] a ## Beginner's guide -Start by reading the gitaly repository's +Start by reading the Gitaly repository's [Beginner's guide to Gitaly contributions](https://gitlab.com/gitlab-org/gitaly/blob/master/doc/beginners_guide.md). -It describes how to setup gitaly, the various components of gitaly and what they do, and how to run its test suites. +It describes how to set up Gitaly, the various components of Gitaly and what they do, and how to run its test suites. ## Developing new Git features @@ -41,14 +41,14 @@ disk access (e.g. Rugged, `git`, `rm -rf`) anywhere outside The process for adding new Gitaly features is: - exploration / prototyping -- design and create a new Gitaly RPC [in gitaly-proto](https://gitlab.com/gitlab-org/gitaly-proto) -- release a new version of gitaly-proto +- design and create a new Gitaly RPC in [`gitaly-proto`](https://gitlab.com/gitlab-org/gitaly-proto) +- release a new version of `gitaly-proto` - write implementation and tests for the RPC [in Gitaly](https://gitlab.com/gitlab-org/gitaly), in Go or Ruby - release a new version of Gitaly - write client code in GitLab CE/EE, GitLab Workhorse or GitLab Shell that calls the new Gitaly RPC These steps often overlap. It is possible to use an unreleased version -of Gitaly and gitaly-proto during testing and development. +of Gitaly and `gitaly-proto` during testing and development. - See the [Gitaly repo](https://gitlab.com/gitlab-org/gitaly/blob/master/CONTRIBUTING.md#development-and-testing-with-a-custom-gitaly-proto) for instructions on writing server side code with an unreleased protocol. - See [below](#running-tests-with-a-locally-modified-version-of-gitaly) for instructions on running GitLab CE tests with a modified version of Gitaly. @@ -58,7 +58,7 @@ of Gitaly and gitaly-proto during testing and development. It is possible to implement and test RPC's in Gitaly using Ruby code, in -[gitaly-ruby](https://gitlab.com/gitlab-org/gitaly/tree/master/ruby). +[`gitaly-ruby`](https://gitlab.com/gitlab-org/gitaly/tree/master/ruby). This should make it easier to contribute for developers who are less comfortable writing Go code. @@ -234,7 +234,7 @@ Here are the steps to gate a new feature in Gitaly behind a feature flag. } ``` -1. Create prometheus metrics: +1. Create Prometheus metrics: ```go var findAllTagsRequests = prometheus.NewCounterVec( diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index 1e230a54023..33dd9dd9b6f 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -62,8 +62,8 @@ file and ask your manager to review and merge. ```yaml projects: - gitlab-ee: reviewer go - gitlab-ce: reviewer go + gitlab: reviewer go + gitlab-foss: reviewer go ``` ## Code style and format diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md index e492b8ea7c9..c7eed880554 100644 --- a/doc/development/gotchas.md +++ b/doc/development/gotchas.md @@ -106,13 +106,16 @@ end 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` +### Alternative: `expect_next_instance_of` or `allow_next_instance_of` Instead of writing: ```ruby # Don't do this: expect_any_instance_of(Project).to receive(:add_import_job) + +# Don't do this: +allow_any_instance_of(Project).to receive(:add_import_job) ``` We could write: @@ -122,10 +125,15 @@ We could write: expect_next_instance_of(Project) do |project| expect(project).to receive(:add_import_job) end + +# Do this: +allow_next_instance_of(Project) do |project| + allow(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: +If we also want to initialize the instance with some particular arguments, we +could also pass it like: ```ruby # Do this: @@ -144,21 +152,19 @@ refresh_service.execute(oldrev, newrev, ref) ## Do not `rescue Exception` -See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception]. +See ["Why is it bad style to `rescue Exception => e` in Ruby?"](https://stackoverflow.com/questions/10048173/why-is-it-bad-style-to-rescue-exception-e-in-ruby). _**Note:** This rule is [enforced automatically by -Rubocop](https://gitlab.com/gitlab-org/gitlab-foss/blob/8-4-stable/.rubocop.yml#L911-914)._ - -[Exception]: http://stackoverflow.com/q/10048173/223897 +Rubocop](https://gitlab.com/gitlab-org/gitlab/blob/8-4-stable/.rubocop.yml#L911-914)._ ## Do not use inline JavaScript in views Using the inline `:javascript` Haml filters comes with a performance overhead. Using inline JavaScript is not a good way to structure your code and should be avoided. -_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/config/initializers/hamlit.rb) +_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab/blob/master/config/initializers/hamlit.rb) in an initializer._ ### Further reading -- Stack Overflow: [Why you should not write inline JavaScript](http://programmers.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting) +- Stack Overflow: [Why you should not write inline JavaScript](https://softwareengineering.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting) diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md index b5e1641abcb..2f067ede70f 100644 --- a/doc/development/i18n/externalization.md +++ b/doc/development/i18n/externalization.md @@ -236,11 +236,11 @@ This makes use of [`Intl.DateTimeFormat`]. - In Ruby/HAML, we have two ways of adding format to dates and times: 1. **Through the `l` helper**, i.e. `l(active_session.created_at, format: :short)`. We have some predefined formats for - [dates](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.7.0/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab-foss/blob/v11.7.0/config/locales/en.yml#L261). + [dates](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/config/locales/en.yml#L262). If you need to add a new format, because other parts of the code could benefit from it, - you'll need to add it to [en.yml](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/config/locales/en.yml) file. + you'll need to add it to [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) file. 1. **Through `strftime`**, i.e. `milestone.start_date.strftime('%b %-d')`. We use `strftime` in case none of the formats - defined on [en.yml](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/config/locales/en.yml) matches the date/time + defined on [en.yml](https://gitlab.com/gitlab-org/gitlab/blob/master/config/locales/en.yml) matches the date/time specifications we need, and if there is no need to add it as a new format because is very particular (i.e. it's only used in a single view). ## Best practices @@ -313,17 +313,22 @@ Developer documentation][mdn]. ## Updating the PO files with the new content -Now that the new content is marked for translation, we need to update the PO -files with the following command: +Now that the new content is marked for translation, we need to update +`locale/gitlab.pot` files with the following command: ```sh bin/rake gettext:regenerate ``` -This command will update the `locale/gitlab.pot` file with the newly externalized +This command will update `locale/gitlab.pot` file with the newly externalized strings and remove any strings that aren't used anymore. You should check this file in. Once the changes are on master, they will be picked up by -[Crowdin](http://translate.gitlab.com) and be presented for translation. +[Crowdin](https://translate.gitlab.com) and be presented for +translation. + +We don't need to check in any changes to the +`locale/[language]/gitlab.po` files. Those will be updated in a [when +translations from Crowdin are merged](merging_translations.md). If there are merge conflicts in the `gitlab.pot` file, you can delete the file and regenerate it using the same command. diff --git a/doc/development/i18n/index.md b/doc/development/i18n/index.md index 5e4923341af..7f59d30f8f9 100644 --- a/doc/development/i18n/index.md +++ b/doc/development/i18n/index.md @@ -29,7 +29,7 @@ See [Externalization for GitLab](externalization.md). ### Translate strings -The translation process is managed at [translate.gitlab.com](https://translate.gitlab.com) +The translation process is managed at <https://translate.gitlab.com> using [Crowdin](https://crowdin.com/). You will need to create an account before you can submit translations. Once you are signed in, select the language you wish to contribute translations to. diff --git a/doc/development/i18n/merging_translations.md b/doc/development/i18n/merging_translations.md index 7a8046ccdd9..f5953cc5f6c 100644 --- a/doc/development/i18n/merging_translations.md +++ b/doc/development/i18n/merging_translations.md @@ -15,8 +15,8 @@ By default Crowdin commits translations with `[skip ci]` in the commit message. This is done to avoid a bunch of pipelines being run. Before merging translations, make sure to trigger a pipeline to validate translations, we have static analysis validating things Crowdin -doesn't do. Create a [new pipeline](https://gitlab.com/gitlab-org/gitlab/pipelines/new) for the -`master-i18n` branch. +doesn't do. Create a new pipeline at `https://gitlab.com/gitlab-org/gitlab/pipelines/new` +(need Developer access permissions) for the `master-i18n` branch. If there are validation errors, the easiest solution is to disapprove the offending string in Crowdin, leaving a comment with what is @@ -53,7 +53,7 @@ or merged. But it won't recreate the `master-i18n` branch every time. To force Crowdin to recreate the branch, close any [open merge request](https://gitlab.com/gitlab-org/gitlab/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&author_username=gitlab-crowdin-bot) and delete the -[`master-18n`](https://gitlab.com/gitlab-org/gitlab/branches/all?utf8=%E2%9C%93&search=master-i18n). +[`master-18n`](https://gitlab.com/gitlab-org/gitlab/-/branches/all?utf8=✓&search=master-i18n). This might be needed when the merge request contains failures that have been fixed on master. diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index db15633fc73..8b3a5d893fe 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -57,7 +57,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 - Hiroyuki Sato - [GitLab](https://gitlab.com/hiroponz), [Crowdin](https://crowdin.com/profile/hiroponz) - - Tomo Dote - [Gitlab](https://gitlab.com/fu7mu4), [Crowdin](https://crowdin.com/profile/fu7mu4) + - Tomo Dote - [GitLab](https://gitlab.com/fu7mu4), [Crowdin](https://crowdin.com/profile/fu7mu4) - Korean - Chang-Ho Cha - [GitLab](https://gitlab.com/changho-cha), [Crowdin](https://crowdin.com/profile/zzazang) - Ji Hun Oh - [GitLab](https://gitlab.com/Baw-Appie), [Crowdin](https://crowdin.com/profile/BawAppie) @@ -128,10 +128,10 @@ are very appreciative of the work done by translators and proofreaders! have previously translated. 1. Your request to become a proofreader will be considered on the merits of - your previous translations by [GitLab team members](https://about.gitlab.com/team/) - or [Core team members](https://about.gitlab.com/core-team/) who are fluent in + your previous translations by [GitLab team members](https://about.gitlab.com/company/team/) + or [Core team members](https://about.gitlab.com/community/core-team/) who are fluent in the language or current proofreaders. - - When a request is made for the first proofreader for a language and there are no [GitLab team members](https://about.gitlab.com/team/) - or [Core team members](https://about.gitlab.com/core-team/) who speak the language, we will request links to previous translation work in other communities or projects. + - When a request is made for the first proofreader for a language and there are no [GitLab team members](https://about.gitlab.com/company/team/) + or [Core team members](https://about.gitlab.com/community/core-team/) who speak the language, we will request links to previous translation work in other communities or projects. [proofreader-src]: https://gitlab.com/gitlab-org/gitlab/blob/master/doc/development/i18n/proofreader.md diff --git a/doc/development/i18n/translation.md b/doc/development/i18n/translation.md index 1793afcd86d..c1a6fd8983c 100644 --- a/doc/development/i18n/translation.md +++ b/doc/development/i18n/translation.md @@ -8,7 +8,7 @@ The first step is to get familiar with Crowdin. ### Sign In -To contribute translations at [translate.gitlab.com](https://translate.gitlab.com) +To contribute translations at <https://translate.gitlab.com> you must create a Crowdin account. You may create a new account or use any of their supported sign in services. @@ -54,7 +54,7 @@ For example in French `OpenedNDaysAgo|Opened` would be translated to Some technical terms should be treated like proper nouns and not be translated. Technical terms that should always be in English are noted in the glossary when -using [translate.gitlab.com](https://translate.gitlab.com). +using <https://translate.gitlab.com>. This helps maintain a logical connection and consistency between tools (e.g. `git` client) and GitLab. diff --git a/doc/development/img/memory_ruby_heap_fragmentation.png b/doc/development/img/memory_ruby_heap_fragmentation.png Binary files differnew file mode 100644 index 00000000000..6567abe58bb --- /dev/null +++ b/doc/development/img/memory_ruby_heap_fragmentation.png diff --git a/doc/development/import_export.md b/doc/development/import_export.md index fcfb9bf4915..38119d9bbd0 100644 --- a/doc/development/import_export.md +++ b/doc/development/import_export.md @@ -31,10 +31,12 @@ Read through the current performance problems using the Import/Export below. Out of memory (OOM) errors are normally caused by the [Sidekiq Memory Killer](../administration/operations/sidekiq_memory_killer.md): ```bash -SIDEKIQ_MEMORY_KILLER_MAX_RSS = 2GB in GitLab.com +SIDEKIQ_MEMORY_KILLER_MAX_RSS = 2000000 +SIDEKIQ_MEMORY_KILLER_HARD_LIMIT_RSS = 3000000 +SIDEKIQ_MEMORY_KILLER_GRACE_TIME = 900 ``` -An import status `started`, and the following sidekiq logs will signal a memory issue: +An import status `started`, and the following Sidekiq logs will signal a memory issue: ```bash WARN: Work still in progress <struct with JID> @@ -96,7 +98,8 @@ importing big projects, using a foreground import: ## Security The Import/Export feature is constantly updated (adding new things to export), however -the code hasn't been refactored in a long time. We should perform a [code audit](https://gitlab.com/gitlab-org/gitlab-foss/issues/42135) +the code hasn't been refactored in a long time. We should perform a code audit (see +[confidential issue](../user/project/issues/confidential_issues.md) `https://gitlab.com/gitlab-org/gitlab/issues/20720`). to make sure its dynamic nature does not increase the number of security concerns. ### Security in the code @@ -309,7 +312,7 @@ module Gitlab class Importer def execute if import_file && check_version! && restorers.all?(&:restore) && overwrite_project - project_tree.restored_project + project else raise Projects::ImportService::Error.new(@shared.errors.join(', ')) end diff --git a/doc/development/interacting_components.md b/doc/development/interacting_components.md index 5e6dc8d460a..3db260d5f85 100644 --- a/doc/development/interacting_components.md +++ b/doc/development/interacting_components.md @@ -14,7 +14,7 @@ change that affects uploads should also be tested against [object storage], which is _not_ enabled by default in [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit). When working on a related feature, make sure to enable and test it -against [Minio](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/object_storage.md). +against [MinIO](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/object_storage.md). See also [File Storage in GitLab](file_storage.md). diff --git a/doc/development/internal_api.md b/doc/development/internal_api.md new file mode 100644 index 00000000000..b08112aacb2 --- /dev/null +++ b/doc/development/internal_api.md @@ -0,0 +1,328 @@ +# Internal API + +The internal API is used by different GitLab components, it can not be +used by other consumers. This documentation is intended for people +working on the GitLab codebase. + +This documentation does not yet include the internal api used by +GitLab pages. + +## Authentication + +These methods are all authenticated using a shared secret. This secret +is stored in a file at the path configured in `config/gitlab.yml` by +default this is in the root of the rails app named +`.gitlab_shell_secret` + +To authenticate using that token, clients read the contents of that +file, and include the token Base64 encoded in a `secret_token` param +or in the `Gitlab-Shared-Secret` header. + +NOTE: **Note:** +The internal api used by GitLab pages uses a different kind of +authentication. + +## Git Authentication + +This is called by Gitaly and GitLab-shell to check access to a +repository. + +When called from GitLab-shell no changes are passed and the internal +API replies with the information needed to pass the request on to +Gitaly. + +When called from Gitaly in a `pre-receive` hook the changes are passed +and those are validated to determine if the push is allowed. + +``` +POST /internal/allowed +``` + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `key_id` | string | no | Id of the SSH-key used to connect to GitLab-shell | +| `username` | string | no | Username from the certificate used to connect to GitLab-Shell | +| `project` | string | no (if `gl_repository` is passed) | Path to the project | +| `gl_repository` | string | no (if `project` is passed) | Path to the project | +| `protocol` | string | yes | SSH when called from GitLab-shell, HTTP or SSH when called from Gitaly | +| `action` | string | yes | Git command being run (`git-upload-pack`, `git-receive-pack`, `git-upload-archive`) | +| `changes` | string | yes | `<oldrev> <newrev> <refname>` when called from Gitaly, The magic string `_any` when called from GitLab Shell | +| `check_ip` | string | no | Ip adress from which call to GitLab Shell was made | + +Example request: + +```sh +curl --request POST --header "Gitlab-Shared-Secret: <Base64 encoded token>" --data "key_id=11&project=gnuwget/wget2&action=git-upload-pack&protocol=ssh" http://localhost:3001/api/v4/internal/allowed +``` + +Example response: + +``` +{ + "status": true, + "gl_repository": "project-3", + "gl_project_path": "gnuwget/wget2", + "gl_id": "user-1", + "gl_username": "root", + "git_config_options": [], + "gitaly": { + "repository": { + "storage_name": "default", + "relative_path": "@hashed/4e/07/4e07408562bedb8b60ce05c1decfe3ad16b72230967de01f640b7e4729b49fce.git", + "git_object_directory": "", + "git_alternate_object_directories": [], + "gl_repository": "project-3", + "gl_project_path": "gnuwget/wget2" + }, + "address": "unix:/Users/bvl/repos/gitlab/gitaly.socket", + "token": null + }, + "gl_console_messages": [] +} +``` + +### Known consumers + +- Gitaly +- GitLab-shell + +## LFS Authentication + +This is the endpoint that gets called from GitLab-shell to provide +information for LFS clients when the repository is accessed over SSH. + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `key_id` | string | no | Id of the SSH-key used to connect to GitLab-shell | +| `username`| string | no | Username from the certificate used to connect to GitLab-Shell | +| `project` | string | no | Path to the project | + +Example request: + +```sh +curl --request POST --header "Gitlab-Shared-Secret: <Base64 encoded token>" --data "key_id=11&project=gnuwget/wget2" http://localhost:3001/api/v4/internal/lfs_authenticate +``` + +``` +{ + "username": "root", + "lfs_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJkYXRhIjp7ImFjdG9yIjoicm9vdCJ9LCJqdGkiOiIyYWJhZDcxZC0xNDFlLTQ2NGUtOTZlMi1mODllYWRiMGVmZTYiLCJpYXQiOjE1NzAxMTc2NzYsIm5iZiI6MTU3MDExNzY3MSwiZXhwIjoxNTcwMTE5NDc2fQ.g7atlBw1QMY7QEBVPE0LZ8ZlKtaRzaMRmNn41r2YITM", + "repository_http_path": "http://localhost:3001/gnuwget/wget2.git", + "expires_in": 1800 +} +``` + +### Known consumers + +- GitLab-shell + +## Authorized Keys Check + +This endpoint is called by the GitLab-shell authorized keys +check. Which is called by OpenSSH for [fast ssh key +lookup](../administration/operations/fast_ssh_key_lookup.md). + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `key` | string | yes | SSH key as passed by OpenSSH to GitLab-shell | + +``` +GET /internal/authorized_keys +``` + +Example request: + +```sh +curl --request GET --header "Gitlab-Shared-Secret: <Base64 encoded secret>""http://localhost:3001/api/v4/internal/authorized_keys?key=<key as passed by OpenSSH>" +``` + +Example response: + +``` +{ + "id": 11, + "title": "admin@example.com", + "key": "ssh-rsa ...", + "created_at": "2019-06-27T15:29:02.219Z" +} +``` + +### Known consumers + +- GitLab-shell + +## Get user for user id or key + +This endpoint is used when a user performs `ssh git@gitlab.com`. It +discovers the user associated with an SSH key. + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `key_id` | integer | no | The id of the SSH key used as found in the authorized-keys file or through the `/authorized_keys` check | +| `username` | string | no | Username of the user being looked up, used by GitLab-shell when authenticating using a certificate | + +``` +GET /internal/discover +``` + +Example request: + +```sh +curl --request GET --header "Gitlab-Shared-Secret: <Base64 encoded secret>" "http://localhost:3001/api/v4/internal/discover?key_id=7" +``` + +Example response: + +``` +{ + "id": 7, + "name": "Dede Eichmann", + "username": "rubi" +} +``` + +### Known consumers + +- GitLab-shell + +## Instance information + +This get's some generic information about the instance. This is used +by Geo nodes to get information about eachother + +``` +GET /internal/check +``` + +Example request: + +```sh +curl --request GET --header "Gitlab-Shared-Secret: <Base64 encoded secret>" "http://localhost:3001/api/v4/internal/check" +``` + +Example response: + +``` +{ + "api_version": "v4", + "gitlab_version": "12.3.0-pre", + "gitlab_rev": "d69c988e6a6", + "redis": true +} +``` + +### Known consumers + +- GitLab Geo +- GitLab-shell's `bin/check` + +## Get new 2FA recovery codes using an SSH key + +This is called from GitLab-shell and allows users to get new 2FA +recovery codes based on their SSH key + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `key_id` | integer | no | The id of the SSH key used as found in the authorized-keys file or through the `/authorized_keys` check | +| `user_id` | integer | no | **Deprecated** User_id for which to generate new recovery codes | + +``` +GET /internal/two_factor_recovery_codes +``` + +Example request: + +```sh +curl --request POST --header "Gitlab-Shared-Secret: <Base64 encoded secret>" --data "key_id=7" http://localhost:3001/api/v4/internal/two_factor_recovery_codes +``` + +Example response: + +``` +{ + "success": true, + "recovery_codes": [ + "d93ee7037944afd5", + "19d7b84862de93dd", + "1e8c52169195bf71", + "be50444dddb7ca84", + "26048c77d161d5b7", + "482d5c03d1628c47", + "d2c695e309ce7679", + "dfb4748afc4f12a7", + "0e5f53d1399d7979", + "af04d5622153b020" + ] +} +``` + +### Known consumers + +- GitLab-shell + +## Incrementing counter on pre-receive + +This is called from the Gitaly hooks increasing the reference counter +for a push that might be accepted. + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `gl_repository` | string | yes | repository identifier for the repository receiving the push | + +``` +POST /internal/pre_receive +``` + +Example request: + +```sh +curl --request POST --header "Gitlab-Shared-Secret: <Base64 encoded secret>" --data "gl_repository=project-7" http://localhost:3001/api/v4/internal/pre_receive +``` + +Example response: + +``` +{ + "reference_counter_increased": true +} +``` + +## Notify Post Receive [UNUSED] ? + +## PostReceive + +Called from Gitaly after a receiving a push. This triggers the +`PostReceive`-worker in sidekiq, processes the passed push options and +builds the response including messages that need to be displayed to +the user. + +| Attribute | Type | Required | Description | +|:----------|:-------|:---------|:------------| +| `identifier` | string | yes | `user-[id]` or `key-[id]` Identifying the user performing the push | +| `gl_repository` | string | yes | identifier of the repository being pushed to | +| `push_options` | [string] | no | array of push options | +| `changes` | string | no | refs to be updated in the push in the format `oldrev newrev refname\n`. | + +``` +POST /internal/post_receive +``` + +Example Request: + +```sh +curl --request POST --header "Gitlab-Shared-Secret: <Base64 encoded secret>" --data "gl_repository=project-7" --data "identifier=user-1" --data "changes=0000000000000000000000000000000000000000 fd9e76b9136bdd9fe217061b497745792fe5a5ee gh-pages\n" http://localhost:3001/api/v4/internal/post_receive +``` + +Example response: + +``` +{ + "messages": [ + { + "message": "Hello from post-receive", + "type": "alert" + } + ], + "reference_counter_decreased": true +} +``` diff --git a/doc/development/issuable-like-models.md b/doc/development/issuable-like-models.md new file mode 100644 index 00000000000..ce19fd77496 --- /dev/null +++ b/doc/development/issuable-like-models.md @@ -0,0 +1,19 @@ +# Issuable-like Rails models utilities + +GitLab Rails codebase contains several models that hold common functionality and behave similarly to an [Issue]. Other +examples of `Issuable`s are [Merge Requests] and [Epics]. + +This guide accumulates guidelines on working with such Rails models. + +## Important text fields + +There are max length constraints for the most important text fields for `Issuable`s: + +- `title`: 255 chars +- `title_html`: 800 chars +- `description`: 1 megabyte +- `description_html`: 5 megabytes + +[Issue]: https://docs.gitlab.com/ee/user/project/issues +[Merge Requests]: https://docs.gitlab.com/ee/user/project/merge_requests +[Epics]: https://docs.gitlab.com/ee/user/group/epics diff --git a/doc/development/kubernetes.md b/doc/development/kubernetes.md index 5265e5b11cd..82aa02ac75d 100644 --- a/doc/development/kubernetes.md +++ b/doc/development/kubernetes.md @@ -29,12 +29,12 @@ We use the [`kubeclient`](https://rubygems.org/gems/kubeclient) gem to perform Kubernetes API calls. As the `kubeclient` gem does not support different API Groups (e.g. `apis/rbac.authorization.k8s.io`) from a single client, we have created a wrapper class, -[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/kubernetes/kube_client.rb) +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/kubernetes/kube_client.rb) that will enable you to achieve this. Selected Kubernetes API groups are currently supported. Do add support for new API groups or methods to -[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/kubernetes/kube_client.rb) +[`Gitlab::Kubernetes::KubeClient`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/kubernetes/kube_client.rb) if you need to use them. New API groups or API group versions can be added to `SUPPORTED_API_GROUPS` - internally, this will create an internal client for that group. New methods can be added as a delegation @@ -44,17 +44,16 @@ to the relevant internal client. All calls to the Kubernetes API must be in a background process. Do not perform Kubernetes API calls within a web request as this will block -unicorn and can easily lead to a Denial Of Service (DoS) attack in GitLab as +Unicorn and can easily lead to a Denial Of Service (DoS) attack in GitLab as the Kubernetes cluster response times are outside of our control. The easiest way to ensure your calls happen a background process is to -delegate any such work to happen in a [sidekiq -worker](sidekiq_style_guide.md). +delegate any such work to happen in a [Sidekiq worker](sidekiq_style_guide.md). There are instances where you would like to make calls to Kubernetes and return the response and as such a background worker does not seem to be a good fit. For such cases you should make use of [reactive -caching](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/models/concerns/reactive_caching.rb). +caching](https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/concerns/reactive_caching.rb). For example: ```ruby @@ -72,7 +71,7 @@ For example: ### Testing We have some Webmock stubs in -[`KubernetesHelpers`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/support/helpers/kubernetes_helpers.rb) +[`KubernetesHelpers`](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/support/helpers/kubernetes_helpers.rb) which can help with mocking out calls to Kubernetes API in your tests. ## Security @@ -87,7 +86,7 @@ a cluster. Mitigation strategies include: 1. Not allowing redirects to attacker controller resources: - [`Kubeclient::KubeClient`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/kubernetes/kube_client.rb#) + [`Kubeclient::KubeClient`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/kubernetes/kube_client.rb#) can be configured to disallow any redirects by passing in `http_max_redirects: 0` as an option. 1. Not exposing error messages: by doing so, we @@ -111,7 +110,7 @@ Logs related to the Kubernetes integration can be found in GDK install, this will be present in `log/kubernetes.log`. Some services such as -[`Clusters::Applications::InstallService`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/services/clusters/applications/install_service.rb#L18) +[`Clusters::Applications::InstallService`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/services/clusters/applications/install_service.rb#L18) rescues `StandardError` which can make it harder to debug issues in an development environment. The current workaround is to temporarily comment out the `rescue` in your local development source. diff --git a/doc/development/licensing.md b/doc/development/licensing.md index 538c8f2039b..40ff604c7c4 100644 --- a/doc/development/licensing.md +++ b/doc/development/licensing.md @@ -46,9 +46,9 @@ More detailed information on how the gem and its commands work is available in t Libraries with the following licenses are acceptable for use: -- [The MIT License][MIT] (the MIT Expat License specifically): The MIT License requires that the license itself is included with all copies of the source. It is a permissive (non-copyleft) license as defined by the Open Source Initiative. -- [LGPL][LGPL] (version 2, version 3): GPL constraints regarding modification and redistribution under the same license are not required of projects using an LGPL library, only upon modification of the LGPL-licensed library itself. -- [Apache 2.0 License][apache-2]: A permissive license that also provides an express grant of patent rights from contributors to users. +- [The MIT License](https://choosealicense.com/licenses/mit/) (the MIT Expat License specifically): The MIT License requires that the license itself is included with all copies of the source. It is a permissive (non-copyleft) license as defined by the Open Source Initiative. +- [LGPL](https://choosealicense.com/licenses/lgpl-3.0/) (version 2, version 3): GPL constraints regarding modification and redistribution under the same license are not required of projects using an LGPL library, only upon modification of the LGPL-licensed library itself. +- [Apache 2.0 License](https://choosealicense.com/licenses/apache-2.0/): A permissive license that also provides an express grant of patent rights from contributors to users. - [Ruby 1.8 License][ruby-1.8]: Dual-licensed under either itself or the GPLv2, defer to the Ruby License itself. Acceptable because of point 3b: "You may distribute the software in object code or binary form, provided that you do at least ONE of the following: b) accompany the distribution with the machine-readable source of the software." - [Ruby 1.9 License][ruby-1.9]: Dual-licensed under either itself or the BSD 2-Clause License, defer to BSD 2-Clause. - [BSD 2-Clause License][BSD-2-Clause]: A permissive (non-copyleft) license as defined by the Open Source Initiative. @@ -62,8 +62,8 @@ Libraries with the following licenses are acceptable for use: Libraries with the following licenses require legal approval for use: -- [GNU GPL][GPL] (version 1, [version 2][GPLv2], [version 3][GPLv3], or any future versions): GPL-licensed libraries cannot be linked to from non-GPL projects. -- [GNU AGPLv3][AGPLv3]: AGPL-licensed libraries cannot be linked to from non-GPL projects. +- [GNU GPL](https://choosealicense.com/licenses/gpl-3.0/) (version 1, [version 2][GPLv2], [version 3][GPLv3], or any future versions): GPL-licensed libraries cannot be linked to from non-GPL projects. +- [GNU AGPLv3](https://choosealicense.com/licenses/agpl-3.0/): AGPL-licensed libraries cannot be linked to from non-GPL projects. - [Open Software License (OSL)][OSL]: is a copyleft license. In addition, the FSF [recommend against its use][OSL-GNU]. - [Facebook BSD + PATENTS][Facebook]: is a 3-clause BSD license with a patent grant that has been deemed [Category X][x-list] by the Apache foundation. - [WTFPL][WTFPL]: is a public domain dedication [rejected by the OSI (3.2)][WTFPL-OSI]. Also has a strong language which is not in accordance with our diversity policy. @@ -109,19 +109,14 @@ Dependencies which are only used in development or test environment are exempt f [CE]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/LICENSE [EE]: https://gitlab.com/gitlab-org/gitlab/blob/master/LICENSE [license_finder]: https://github.com/pivotal/LicenseFinder -[MIT]: http://choosealicense.com/licenses/mit/ -[LGPL]: http://choosealicense.com/licenses/lgpl-3.0/ -[apache-2]: http://choosealicense.com/licenses/apache-2.0/ [ruby-1.8]: https://github.com/ruby/ruby/blob/ruby_1_8_6/COPYING [ruby-1.9]: https://www.ruby-lang.org/en/about/license.txt [BSD-2-Clause]: https://opensource.org/licenses/BSD-2-Clause [BSD-3-Clause]: https://opensource.org/licenses/BSD-3-Clause [ISC]: https://opensource.org/licenses/ISC [CC0]: https://creativecommons.org/publicdomain/zero/1.0/ -[GPL]: http://choosealicense.com/licenses/gpl-3.0/ [GPLv2]: http://www.gnu.org/licenses/gpl-2.0.txt [GPLv3]: http://www.gnu.org/licenses/gpl-3.0.txt -[AGPLv3]: http://choosealicense.com/licenses/agpl-3.0/ [GNU-GPL-FAQ]: http://www.gnu.org/licenses/gpl-faq.html#IfLibraryIsGPL [OSI-GPL]: https://opensource.org/faq#linking-proprietary-code [OSL]: https://opensource.org/licenses/OSL-3.0 diff --git a/doc/development/merge_request_performance_guidelines.md b/doc/development/merge_request_performance_guidelines.md index 6f1baad3a2d..4456e5e6d18 100644 --- a/doc/development/merge_request_performance_guidelines.md +++ b/doc/development/merge_request_performance_guidelines.md @@ -41,7 +41,7 @@ about the impact. Sometimes it's hard to assess the impact of a merge request. In this case you should ask one of the merge request reviewers to review your changes. You can -find a list of these reviewers at <https://about.gitlab.com/team/>. A reviewer +find a list of these reviewers at <https://about.gitlab.com/company/team/>. A reviewer in turn can request a performance specialist to review the changes. ## Query Counts @@ -68,7 +68,7 @@ end This will end up running one query for every object to update. This code can easily overload a database given enough rows to update or many instances of this code running in parallel. This particular problem is known as the -["N+1 query problem"](http://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations). You can write a test with [QueryRecoder](query_recorder.md) to detect this and prevent regressions. +["N+1 query problem"](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations). You can write a test with [QueryRecoder](query_recorder.md) to detect this and prevent regressions. In this particular case the workaround is fairly easy: @@ -171,4 +171,4 @@ Caching data per transaction can be done using [RequestStore](https://github.com/steveklabnik/request_store) (use `Gitlab::SafeRequestStore` to avoid having to remember to check `RequestStore.active?`). Caching data in Redis can be done using [Rails' caching -system](http://guides.rubyonrails.org/caching_with_rails.html). +system](https://guides.rubyonrails.org/caching_with_rails.html). diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 5fded9e3aed..20d705136b2 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -286,6 +286,48 @@ For a small table (such as an empty one or one with less than `1,000` records), use `add_column` and `change_column_default` in a single-transaction migration, combining it with other operations that don't require `disable_ddl_transaction!`. +## Changing the column default + +One might think that changing a default column with `change_column_default` is an +expensive and disruptive operation for larger tables, but in reality it's not. + +Take the following migration as an example: + +```ruby +class DefaultRequestAccessGroups < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + change_column_default :namespaces, :request_access_enabled, true + end + + def down + change_column_default :namespaces, :request_access_enabled, false + end +end +``` + +Migration above changes the default column value of one of our largest +tables: `namespaces`. This can be translated to: + +```sql +ALTER TABLE namespaces +ALTER COLUMN request_access_enabled +DEFAULT false +``` + +In this particular case, the default value exists and we're just changing the metadata for +`request_access_enabled` column, which does not imply a rewrite of all the existing records +in the `namespaces` table. Only when creating a new column with a default, all the records are going be rewritten. + +NOTE: **Note:** A faster [ALTER TABLE ADD COLUMN with a non-null default](https://www.depesz.com/2018/04/04/waiting-for-postgresql-11-fast-alter-table-add-column-with-a-non-null-default/) +was introduced on PostgresSQL 11.0, removing the need of rewritting the table when a new column with a default value is added. + +For the reasons mentioned above, it's safe to use `change_column_default` in a single-transaction migration +without requiring `disable_ddl_transaction!`. + ## Updating an existing column To update an existing column to a particular value, you can use @@ -375,6 +417,7 @@ timestamps with timezones: - `add_timestamps_with_timezone` - `timestamps_with_timezone` +- `datetime_with_timezone` This ensures all timestamps have a time zone specified. This, in turn, means existing timestamps won't suddenly use a different timezone when the system's @@ -406,10 +449,7 @@ end ## Testing -Make sure that your migration works for databases with data. An -empty database does not guarantee that your migration is correct. - -Make sure your migration can be reversed. +See the [Testing Rails migrations](testing_guide/testing_migrations_guide.md) style guide. ## Data migration @@ -481,5 +521,5 @@ by an integer. For example: `users` would turn into `users0` ### Moving migrations from EE to CE When migrations need to be moved from GitLab Enterprise Edition to GitLab Community Edition, -a migration file should be moved from `ee/db/{post_,}migrate` directory in the `gitlab-ee` project to `db/{post_,}migrate` directory in the `gitlab-ce` project. This way +a migration file should be moved from `ee/db/{post_,}migrate` directory in the `gitlab` project to `db/{post_,}migrate` directory in the `gitlab-foss` project. This way the schema number remains intact, there is no need to modify old migrations, and proper columns, tables or data are added in the Community Edition. diff --git a/doc/development/namespaces_storage_statistics.md b/doc/development/namespaces_storage_statistics.md index f8aea10097d..648f189a826 100644 --- a/doc/development/namespaces_storage_statistics.md +++ b/doc/development/namespaces_storage_statistics.md @@ -14,11 +14,11 @@ storage consumed by a group, and allow easy management. ## Problem In GitLab, we update the project storage statistics through a -[callback](https://gitlab.com/gitlab-org/gitlab-foss/blob/v12.2.0.pre/app/models/project.rb#L90) +[callback](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/app/models/project.rb#L97) every time the project is saved. The summary of those statistics per namespace is then retrieved -by [`Namespaces#with_statistics`](https://gitlab.com/gitlab-org/gitlab-foss/blob/v12.2.0.pre/app/models/namespace.rb#L70) scope. Analyzing this query we noticed that: +by [`Namespaces#with_statistics`](https://gitlab.com/gitlab-org/gitlab/blob/4ab54c2233e91f60a80e5b6fa2181e6899fdcc3e/app/models/namespace.rb#L70) scope. Analyzing this query we noticed that: - It takes up to `1.2` seconds for namespaces with over `15k` projects. - It can't be analyzed with [ChatOps](chatops_on_gitlabcom.md), as it times out. diff --git a/doc/development/new_fe_guide/development/components.md b/doc/development/new_fe_guide/development/components.md index cebdc87eab9..7cccd4b5b1b 100644 --- a/doc/development/new_fe_guide/development/components.md +++ b/doc/development/new_fe_guide/development/components.md @@ -7,7 +7,7 @@ We have a lot of graphing libraries in our codebase to render graphs. In an effo We chose D3 as our library going forward because of the following features: - [Tree shaking webpack capabilities](https://github.com/d3/d3/blob/master/CHANGES.md#changes-in-d3-40). -- [Compatible with vue.js as well as vanilla javascript](https://github.com/d3/d3/blob/master/CHANGES.md#changes-in-d3-40). +- [Compatible with vue.js as well as vanilla JavaScript](https://github.com/d3/d3/blob/master/CHANGES.md#changes-in-d3-40). D3 is very popular across many projects outside of GitLab: diff --git a/doc/development/new_fe_guide/development/performance.md b/doc/development/new_fe_guide/development/performance.md index d41239693bf..4a85c04d8cf 100644 --- a/doc/development/new_fe_guide/development/performance.md +++ b/doc/development/new_fe_guide/development/performance.md @@ -2,9 +2,9 @@ ## Monitoring -We have a performance dashboard available in one of our [grafana instances](https://dashboards.gitlab.net/d/1EBTz3Dmz/sitespeed-page-summary?orgId=1). This dashboard automatically aggregates metric data from [sitespeed.io](https://www.sitespeed.io/) every 6 hours. These changes are displayed after a set number of pages are aggregated. +We have a performance dashboard available in one of our [Grafana instances](https://dashboards.gitlab.net/d/1EBTz3Dmz/sitespeed-page-summary?orgId=1). This dashboard automatically aggregates metric data from [sitespeed.io](https://www.sitespeed.io/) every 6 hours. These changes are displayed after a set number of pages are aggregated. -These pages can be found inside a text file in the gitlab-build-images [repository](https://gitlab.com/gitlab-org/gitlab-build-images) called [gitlab.txt](https://gitlab.com/gitlab-org/gitlab-build-images/blob/master/scripts/gitlab.txt) +These pages can be found inside a text file in the [`gitlab-build-images` repository](https://gitlab.com/gitlab-org/gitlab-build-images) called [`gitlab.txt`](https://gitlab.com/gitlab-org/gitlab-build-images/blob/master/scripts/gitlab.txt) Any frontend engineer can contribute to this dashboard. They can contribute by adding or removing urls of pages from this text file. Please have a [frontend monitoring expert](https://about.gitlab.com/company/team/) review your changes before assigning to a maintainer of the `gitlab-build-images` project. The changes will go live on the next scheduled run after the changes are merged into `master`. There are 3 recommended high impact metrics to review on each page: diff --git a/doc/development/new_fe_guide/modules/dirty_submit.md b/doc/development/new_fe_guide/modules/dirty_submit.md index 2ad6b8b60a3..dd7d5d61c4d 100644 --- a/doc/development/new_fe_guide/modules/dirty_submit.md +++ b/doc/development/new_fe_guide/modules/dirty_submit.md @@ -8,7 +8,7 @@ Prevent submitting forms with no changes. Currently handles `input`, `textarea` and `select` elements. -Also, see [the code](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/assets/javascripts/dirty_submit/) +Also, see [the code](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/javascripts/dirty_submit/) within the GitLab project. ## Usage diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md index b742d567f41..d31edcb372d 100644 --- a/doc/development/new_fe_guide/style/javascript.md +++ b/doc/development/new_fe_guide/style/javascript.md @@ -188,8 +188,8 @@ disabled due to legacy compatibility reasons but they are in the process of bein Do not disable specific ESLint rules. Due to technical debt, you may disable the following rules only if you are invoking/instantiating existing code modules. -- [no-new](http://eslint.org/docs/rules/no-new) -- [class-method-use-this](http://eslint.org/docs/rules/class-methods-use-this) +- [no-new](https://eslint.org/docs/rules/no-new) +- [class-method-use-this](https://eslint.org/docs/rules/class-methods-use-this) > Note: Disable these rules on a per line basis. This makes it easier to refactor > in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`. diff --git a/doc/development/performance.md b/doc/development/performance.md index 39fcc8ff806..786b590ec70 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -251,6 +251,36 @@ These results can also be placed into a PostgreSQL database by setting the `RSPEC_PROFILING_POSTGRES_URL` variable. This is used to profile the test suite when running in the CI environment. +## Memory profiling + +One of the reasons of the increased memory footprint could be Ruby memory fragmentation. + +To diagnose it, you can visualize Ruby heap as described in [this post by Aaron Patterson](https://tenderlovemaking.com/2017/09/27/visualizing-your-ruby-heap.html). + +To start, you want to dump the heap of the process you are investigating to a JSON file. + +You need to run the command inside the process you are exploring, you may do that with `rbtrace`. +`rbtrace` is already present in GitLab `Gemfile`, you just need to require it. +It could be achieved running webserver or Sidekiq with the environment variable set to `ENABLE_RBTRACE=1`. + +To get the heap dump: + +```ruby +bundle exec rbtrace -p <PID> -e 'File.open("heap.json", "wb") { |t| ObjectSpace.dump_all(output: t) }' +``` + +Having the JSON, you finally could render a picture using the script [provided by Aaron](https://gist.github.com/tenderlove/f28373d56fdd03d8b514af7191611b88) or similar: + +```sh +ruby heapviz.rb heap.json +``` + +Fragmented Ruby heap snapshot could look like this: + + + +Memory fragmentation could be reduced by tuning GC parameters as described in [this post by Nate Berkopec](https://www.speedshop.co/2017/12/04/malloc-doubles-ruby-memory.html), which should be considered as a tradeoff, as it may affect overall performance of memory allocation and GC cycles. + ## Importance of Changes When working on performance improvements, it's important to always ask yourself diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 448fb0f9f5a..5954de03db4 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -27,6 +27,7 @@ The current stages are: - `review`: This stage includes jobs that deploy the GitLab and Docs Review Apps. - `qa`: This stage includes jobs that perform QA tasks against the Review App that is deployed in the previous stage. +- `notification`: This stage includes jobs that sends notifications about pipeline status. - `post-test`: This stage includes jobs that build reports or gather data from the previous stages' jobs (e.g. coverage, Knapsack metadata etc.). - `pages`: This stage includes a job that deploys the various reports as @@ -37,7 +38,7 @@ The current stages are: ## Default image The default image is currently -`dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.6.3-golang-1.11-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33`. +`gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.3-golang-1.11-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33`. It includes Ruby 2.6.3, Go 1.11, Git 2.22, Chrome 73, Node 12, Yarn 1.16, PostgreSQL 9.6, and Graphics Magick 1.3.33. @@ -57,11 +58,10 @@ each pipeline includes the following [variables](../ci/variables/README.md): - `RAILS_ENV: "test"` - `NODE_ENV: "test"` - `SIMPLECOV: "true"` -- `GIT_DEPTH: "20"` +- `GIT_DEPTH: "50"` - `GIT_SUBMODULE_STRATEGY: "none"` - `GET_SOURCES_ATTEMPTS: "3"` - `KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` -- `EE_KNAPSACK_RSPEC_SUITE_REPORT_PATH: knapsack/${CI_PROJECT_NAME}/rspec_report-master-ee.json` - `FLAKY_RSPEC_SUITE_REPORT_PATH: rspec_flaky/report-suite.json` - `BUILD_ASSETS_IMAGE: "false"` - `ES_JAVA_OPTS: "-Xms256m -Xmx256m"` @@ -92,9 +92,17 @@ These common definitions are: for `master` and auto-deploy branches. - `.only-review-schedules`: Same as `.only-review` but also restrict a job to only run for [schedules](../user/project/pipelines/schedules.md). -- `.use-pg`: Allows a job to use the `postgres:9.6.14` and `redis:alpine` services. -- `.use-pg-10`: Allows a job to use the `postgres:10.9` and `redis:alpine` services. +- `.only-canonical-schedules`: Only creates a job for scheduled pipelines in + the `gitlab-org/gitlab` and `gitlab-org/gitlab-foss` projects +- `.use-pg9`: Allows a job to use the `postgres:9.6` and `redis:alpine` services. +- `.use-pg10`: Allows a job to use the `postgres:10.9` and `redis:alpine` services. +- `.use-pg9-ee`: Same as `.use-pg9` but also use the + `docker.elastic.co/elasticsearch/elasticsearch:5.6.12` services. +- `.use-pg10-ee`: Same as `.use-pg10` but also use the + `docker.elastic.co/elasticsearch/elasticsearch:5.6.12` services. - `.only-ee`: Only creates a job for the `gitlab` project. +- `.only-ee-as-if-foss`: Same as `.only-ee` but simulate the FOSS project by + setting the `FOSS_ONLY='1'` environment variable. ## Changes detection @@ -107,6 +115,7 @@ from a commit or MR by extending from the following CI definitions: - `.only-qa-changes`: Allows a job to only be created upon QA-related changes. - `.only-docs-changes`: Allows a job to only be created upon docs-related changes. - `.only-code-qa-changes`: Allows a job to only be created upon code-related or QA-related changes. +- `.only-graphql-changes`: Allows a job to only be created upon graphql-related changes. **See <https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/global.gitlab-ci.yml> for the list of exact patterns.** @@ -119,7 +128,7 @@ execute jobs out of order for the following jobs: ```mermaid graph RL; A[setup-test-env]; - B["gitlab:assets:compile<br/>(master only)"]; + B["gitlab:assets:compile pull-push-cache<br/>(master only)"]; C[gitlab:assets:compile pull-cache]; D["cache gems<br/>(master and tags only)"]; E[review-build-cng]; @@ -128,48 +137,51 @@ graph RL; G2["schedule:review-deploy<br/>(master only)"]; H[karma]; I[jest]; - J["compile-assets<br/>(master only)"]; + J["compile-assets pull-push-cache<br/>(master only)"]; K[compile-assets pull-cache]; L[webpack-dev-server]; M[coverage]; N[pages]; O[static-analysis]; - P["package-and-qa-manual:master<br/>(master schedule only)"]; + P["schedule:package-and-qa<br/>(master schedule only)"]; Q[package-and-qa]; R[package-and-qa-manual]; + S["RSpec<br/>(e.g. rspec unit pg9)"] + T[retrieve-tests-metadata]; subgraph "`prepare` stage" A F - J K + J + T end subgraph "`test` stage" B --> |needs| A; C --> |needs| A; D --> |needs| A; - H -.-> |depends on| A; - H -.-> |depends on| J; - H -.-> |depends on| K; - I -.-> |depends on| A; - I -.-> |depends on| J; - I -.-> |depends on| K; - L -.-> |depends on| A; - L -.-> |depends on| J; - L -.-> |depends on| K; + H -.-> |needs and depends on| A; + H -.-> |needs and depends on| K; + I -.-> |needs and depends on| A; + I -.-> |needs and depends on| K; + L -.-> |needs and depends on| A; + L -.-> |needs and depends on| K; + O -.-> |needs and depends on| A; + O -.-> |needs and depends on| K; + S -.-> |needs and depends on| A; + S -.-> |needs and depends on| K; + S -.-> |needs and depends on| T; downtime_check --> |needs and depends on| A; db:* --> |needs| A; gitlab:setup --> |needs| A; - O -.-> |depends on| A; - O -.-> |depends on| B; - O -.-> |depends on| C; downtime_check --> |needs and depends on| A; + graphql-docs-verify --> |needs| A; end subgraph "`review-prepare` stage" E --> |needs| C; - X["schedule:review-build-cng<br/>(master schedule only)"] --> |needs| B; + X["schedule:review-build-cng<br/>(master schedule only)"] --> |needs| C; end subgraph "`review` stage" @@ -182,13 +194,18 @@ subgraph "`qa` stage" Q --> |needs| F; R --> |needs| C; R --> |needs| F; - P --> |needs| B; + P --> |needs| C; P --> |needs| F; - review-qa-smoke -.-> |depends on| G; - review-qa-all -.-> |depends on| G; - review-qa-performance -.-> |depends on| G; - X2["schedule:review-performance<br/>(master only)"] -.-> |depends on| G2; - dast -.-> |depends on| G; + review-qa-smoke -.-> |needs and depends on| G; + review-qa-all -.-> |needs and depends on| G; + review-performance -.-> |needs and depends on| G; + X2["schedule:review-performance<br/>(master only)"] -.-> |needs and depends on| G2; + dast -.-> |needs and depends on| G; + end + +subgraph "`notification` stage" + NOTIFICATION1["schedule:package-and-qa:notify-success<br>(on_success)"] -.-> |needs| P; + NOTIFICATION2["schedule:package-and-qa:notify-failure<br>(on_failure)"] -.-> |needs| P; end subgraph "`post-test` stage" @@ -196,7 +213,7 @@ subgraph "`post-test` stage" end subgraph "`pages` stage" - N -.-> |depends on| B; + N -.-> |depends on| C; N -.-> |depends on| H; N -.-> |depends on| M; end diff --git a/doc/development/profiling.md b/doc/development/profiling.md index e1d1d2e33fa..04897e770f8 100644 --- a/doc/development/profiling.md +++ b/doc/development/profiling.md @@ -86,8 +86,8 @@ that builds on this to add some additional niceties, such as allowing configuration with a single Yaml file for multiple URLs, and uploading of the profile and log output to S3. -For GitLab.com, you can find the latest results here: -<http://redash.gitlab.com/dashboard/gitlab-profiler-statistics> +For GitLab.com, you can find the latest results here (restricted to GitLab Team members only): +`https://redash.gitlab.com/dashboard/gitlab-profiler-statistics` ## Sherlock diff --git a/doc/development/projections.md b/doc/development/projections.md new file mode 100644 index 00000000000..9d5702da530 --- /dev/null +++ b/doc/development/projections.md @@ -0,0 +1,34 @@ +# Projections + +Projections are a way to define relations between files. Every file can have a +"related" or "alternate" file. It's common to consider spec files to be +"alternate" files to source files. + +## How to use it + +- Install an editor plugin that consumes projections +- Copy `.projections.json.example` to `.projections.json` + +## How to customize it + +You can find a basic list of projection options in +[projectionist.txt](https://github.com/tpope/vim-projectionist/blob/master/doc/projectionist.txt) + +## Which plugins can I use + +- vim + - [vim-projectionist](https://github.com/tpope/vim-projectionist) +- VSCode + - [Alternate File](https://marketplace.visualstudio.com/items?itemName=will-wow.vscode-alternate-file) + - [projectionist](https://github.com/jarsen/projectionist) + - [jumpto](https://github.com/gmdayley/jumpto) +- Atom + - [projectionist-atom](https://atom.io/packages/projectionist-atom) +- Command-line + - [projectionist](https://github.com/glittershark/projectionist) + +## History + +This started as a +[plugin for vim by tpope](https://github.com/tpope/vim-projectionist) +It has since become editor-agnostic and ported to most modern editors. diff --git a/doc/development/prometheus_metrics.md b/doc/development/prometheus_metrics.md index eecce9f4f11..b479c053862 100644 --- a/doc/development/prometheus_metrics.md +++ b/doc/development/prometheus_metrics.md @@ -2,7 +2,7 @@ ## Adding to the library -We strive to support the 2-4 most important metrics for each common system service that supports Prometheus. If you are looking for support for a particular exporter which has not yet been added to the library, additions can be made [to the `common_metrics.yml`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/config/prometheus/common_metrics.yml) file. +We strive to support the 2-4 most important metrics for each common system service that supports Prometheus. If you are looking for support for a particular exporter which has not yet been added to the library, additions can be made [to the `common_metrics.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/config/prometheus/common_metrics.yml) file. ### Query identifier diff --git a/doc/development/python_guide/index.md b/doc/development/python_guide/index.md index 47d9d96766c..d898351345d 100644 --- a/doc/development/python_guide/index.md +++ b/doc/development/python_guide/index.md @@ -27,7 +27,7 @@ To install `pyenv` on Linux, you can run the command below: curl https://pyenv.run | bash ``` -Alternatively, you may find `pypenv` available as a system package via your distro package manager. +Alternatively, you may find `pyenv` available as a system package via your distro package manager. You can read more about it in: <https://github.com/pyenv/pyenv-installer#prerequisites>. diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index 3ed75c87f6f..81970b45bbd 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -2,7 +2,7 @@ QueryRecorder is a tool for detecting the [N+1 queries problem](https://guides.rubyonrails.org/active_record_querying.html#eager-loading-associations) from tests. -> Implemented in [spec/support/query_recorder.rb](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/support/helpers/query_recorder.rb) via [9c623e3e](https://gitlab.com/gitlab-org/gitlab-foss/commit/9c623e3e5d7434f2e30f7c389d13e5af4ede770a) +> Implemented in [spec/support/query_recorder.rb](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/support/helpers/query_recorder.rb) via [9c623e3e](https://gitlab.com/gitlab-org/gitlab-foss/commit/9c623e3e5d7434f2e30f7c389d13e5af4ede770a) As a rule, merge requests [should not increase query counts](merge_request_performance_guidelines.md#query-counts). If you find yourself adding something like `.includes(:author, :assignee)` to avoid having `N+1` queries, consider using QueryRecorder to enforce this with a test. Without this, a new feature which causes an additional model to be accessed will silently reintroduce the problem. diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md index a144bf70a86..a6d3c008686 100644 --- a/doc/development/rake_tasks.md +++ b/doc/development/rake_tasks.md @@ -143,12 +143,6 @@ This will compile and minify all JavaScript and CSS assets and copy them along with all other frontend assets (images, fonts, etc) into `/public/assets` where they can be easily inspected. -## Generate API documentation for project services (e.g. Slack) - -``` -bundle exec rake services:doc -``` - ## Updating Emoji Aliases To update the Emoji aliases file (used for Emoji autocomplete) you must run the @@ -226,3 +220,26 @@ bundle exec rake db:obsolete_ignored_columns ``` Feel free to remove their definitions from their `ignored_columns` definitions. + +## Update GraphQL Documentation + +To generate GraphQL documentation based on the GitLab schema, run: + +```shell +bundle exec rake gitlab:graphql:compile_docs +``` + +In its current state, the rake task: + +- Generates output for GraphQL objects. +- Places the output at `doc/api/graphql/reference/index.md`. + +This uses some features from `graphql-docs` gem like its schema parser and helper methods. +The docs generator code comes from our side giving us more flexibility, like using Haml templates and generating Markdown files. + +To edit the template used, please take a look at `lib/gitlab/graphql/docs/templates/default.md.haml`. +The actual renderer is at `Gitlab::Graphql::Docs::Renderer`. + +`@parsed_schema` is an instance variable that the `graphql-docs` gem expects to have available. +`Gitlab::Graphql::Docs::Helper` defines the `object` method we currently use. This is also where you +should implement any new methods for new types you'd like to display. diff --git a/doc/development/repository_mirroring.md b/doc/development/repository_mirroring.md index dc51bf80e92..8521d6fcd30 100644 --- a/doc/development/repository_mirroring.md +++ b/doc/development/repository_mirroring.md @@ -2,10 +2,9 @@ ## Deep Dive -In December 2018, Tiago Botelho hosted a [Deep Dive] on GitLab's [Pull Repository Mirroring functionality] to share his domain specific knowledge with anyone who may work in this part of the code base in the future. You can find the [recording on YouTube], and the slides on [Google Slides] and in [PDF]. Everything covered in this deep dive was accurate as of GitLab 11.6, and while specific details may have changed since then, it should still serve as a good introduction. +In December 2018, Tiago Botelho hosted a [Deep Dive] on GitLab's [Pull Repository Mirroring functionality] to share his domain specific knowledge with anyone who may work in this part of the code base in the future. You can find the [recording on YouTube], and the slides in [PDF]. Everything covered in this deep dive was accurate as of GitLab 11.6, and while specific details may have changed since then, it should still serve as a good introduction. [Deep Dive]: https://gitlab.com/gitlab-org/create-stage/issues/1 [Pull Repository Mirroring functionality]: ../workflow/repository_mirroring.md#pulling-from-a-remote-repository-starter [recording on YouTube]: https://www.youtube.com/watch?v=sSZq0fpdY-Y -[Google Slides]: https://docs.google.com/presentation/d/17BTT6M6RyNRckV4wTt-dr07nIfBvD325_xVBoLtSoPM/edit?usp=sharing [PDF]: https://gitlab.com/gitlab-org/create-stage/uploads/8693404888a941fd851f8a8ecdec9675/Gitlab_Create_-_Pull_Mirroring_Deep_Dive.pdf diff --git a/doc/development/routing.md b/doc/development/routing.md index 0b95477974b..e0741e78821 100644 --- a/doc/development/routing.md +++ b/doc/development/routing.md @@ -57,9 +57,9 @@ client or other software requires something different. Examples: ``` -gitlab-org/gitlab-ce/-/activity -gitlab-org/gitlab-ce/-/jobs/123 -gitlab-org/gitlab-ce/-/settings/repository +gitlab-org/gitlab/-/activity +gitlab-org/gitlab/-/jobs/123 +gitlab-org/gitlab/-/settings/repository gitlab-org/serverless/runtimes/-/settings/repository ``` diff --git a/doc/development/shell_commands.md b/doc/development/shell_commands.md index 1300c99622e..aa326cbdd34 100644 --- a/doc/development/shell_commands.md +++ b/doc/development/shell_commands.md @@ -5,9 +5,9 @@ These guidelines are meant to make your code more reliable _and_ secure. ## References -- [Google Ruby Security Reviewer's Guide](https://code.google.com/p/ruby-security/wiki/Guide) +- [Google Ruby Security Reviewer's Guide](https://code.google.com/archive/p/ruby-security/wikis/Guide.wiki) - [OWASP Command Injection](https://www.owasp.org/index.php/Command_Injection) -- [Ruby on Rails Security Guide Command Line Injection](http://guides.rubyonrails.org/security.html#command-line-injection) +- [Ruby on Rails Security Guide Command Line Injection](https://guides.rubyonrails.org/security.html#command-line-injection) ## Use File and FileUtils instead of shell commands @@ -87,7 +87,7 @@ $ cat -- -l hello ``` -In the GitLab codebase, we avoid the option/argument ambiguity by _always_ using `--`. +In the GitLab codebase, we avoid the option/argument ambiguity by _always_ using `--` for commands that support it. ```ruby # Wrong diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index 8e268224c98..d52a3e652e3 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -27,7 +27,7 @@ While different workers cannot share a queue, they can share a queue namespace. Defining a queue namespace for a worker makes it possible to start a Sidekiq process that automatically handles jobs for all workers in that namespace, without needing to explicitly list all their queue names. If, for example, all -workers that are managed by sidekiq-cron use the `cronjob` queue namespace, we +workers that are managed by `sidekiq-cron` use the `cronjob` queue namespace, we can spin up a Sidekiq process specifically for these kinds of scheduled jobs. If a new worker using the `cronjob` namespace is added later on, the Sidekiq process will automatically pick up jobs for that worker too (after having been @@ -61,6 +61,56 @@ the extra jobs will take resources away from jobs from workers that were already there, if the resources available to the Sidekiq process handling the namespace are not adjusted appropriately. +## Feature Categorization + +Each Sidekiq worker, or one of its ancestor classes, must declare a +`feature_category` attribute. This attribute maps each worker to a feature +category. This is done for error budgeting, alert routing, and team attribution +for Sidekiq workers. + +The declaration uses the `feature_category` class method, as shown below. + +```ruby +class SomeScheduledTaskWorker + include ApplicationWorker + + # Declares that this feature is part of the + # `continuous_integration` feature category + feature_category :continuous_integration + + # ... +end +``` + +The list of value values can be found in the file `config/feature_categories.yml`. +This file is, in turn generated from the [`stages.yml` from the GitLab Company Handbook +source](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml). + +### Updating `config/feature_categories.yml` + +Occassionally new features will be added to GitLab stages. When this occurs, you +can automatically update `config/feature_categories.yml` by running +`scripts/update-feature-categories`. This script will fetch and parse +[`stages.yml`](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/stages.yml) +and generare a new version of the file, which needs to be checked into source control. + +### Excluding Sidekiq workers from feature categorization + +A few Sidekiq workers, that are used across all features, cannot be mapped to a +single category. These should be declared as such using the `feature_category_not_owned!` + declaration, as shown below: + +```ruby +class SomeCrossCuttingConcernWorker + include ApplicationWorker + + # Declares that this worker does not map to a feature category + feature_category_not_owned! + + # ... +end +``` + ## Tests Each Sidekiq worker must be tested using RSpec, just like any other class. These diff --git a/doc/development/sql.md b/doc/development/sql.md index 2584dcfb4ca..8a8204ffe87 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -74,7 +74,7 @@ USING GIN(column_name gin_trgm_ops); ``` The key here is the `GIN(column_name gin_trgm_ops)` part. This creates a [GIN -index][gin-index] with the operator class set to `gin_trgm_ops`. These indexes +index](https://www.postgresql.org/docs/current/gin.html) with the operator class set to `gin_trgm_ops`. These indexes _can_ be used by `ILIKE` / `LIKE` and can lead to greatly improved performance. One downside of these indexes is that they can easily get quite large (depending on the amount of data indexed). @@ -247,8 +247,6 @@ WHERE EXISTS ( ) ``` -[gin-index]: http://www.postgresql.org/docs/current/static/gin.html - ## `.find_or_create_by` is not atomic The inherent pattern with methods like `.find_or_create_by` and diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index a8cbf3aaa5b..32e079f915c 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -51,7 +51,7 @@ bundle exec rspec spec/[path]/[to]/[spec].rb methods. - Use `context` to test branching logic. - Try to match the ordering of tests to the ordering within the class. -- Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines +- Try to follow the [Four-Phase Test](https://thoughtbot.com/blog/four-phase-test) pattern, using newlines to separate phases. - Use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'` - Don't assert against the absolute value of a sequence-generated attribute (see @@ -62,8 +62,6 @@ bundle exec rspec spec/[path]/[to]/[spec].rb use a Capyabara matcher beforehand (e.g. `find('.js-foo')`) to ensure the element actually exists. - Use `focus: true` to isolate parts of the specs you want to run. -[four-phase-test]: https://robots.thoughtbot.com/four-phase-test - ### System / Feature tests NOTE: **Note:** Before writing a new system test, [please consider **not** @@ -115,7 +113,7 @@ Finished in 34.51 seconds (files took 0.76702 seconds to load) 1 example, 0 failures ``` -Note: `live_debug` only works on javascript enabled specs. +Note: `live_debug` only works on JavaScript enabled specs. #### Run `:js` spec in a visible browser @@ -160,7 +158,7 @@ really fast since: - Gems loading is skipped - Rails app boot is skipped -- gitlab-shell and Gitaly setup are skipped +- GitLab Shell and Gitaly setup are skipped - Test repositories setup are skipped `fast_spec_helper` also support autoloading classes that are located inside the @@ -185,7 +183,7 @@ instead of 30+ seconds in case of a regular `spec_helper`. ### `let` variables GitLab's RSpec suite has made extensive use of `let`(along with it strict, non-lazy -version `let!`) variables to reduce duplication. However, this sometimes [comes at the cost of clarity][lets-not], +version `let!`) variables to reduce duplication. However, this sometimes [comes at the cost of clarity](https://thoughtbot.com/blog/lets-not), so we need to set some guidelines for their use going forward: - `let!` variables are preferable to instance variables. `let` variables @@ -204,10 +202,51 @@ so we need to set some guidelines for their use going forward: order is required, otherwise `let` will suffice. Remember that `let` is lazy and won't be evaluated until it is referenced. -[lets-not]: https://robots.thoughtbot.com/lets-not +### Common test setup + +In some cases, there is no need to recreate the same object for tests +again for each example. For example, a project and a guest of that project +is needed to test issues on the same project, one project and user will do for the entire file. +This can be achieved by using +[`let_it_be`](https://test-prof.evilmartians.io/#/let_it_be) variables and the +[`before_all`](https://test-prof.evilmartians.io/#/before_all) hook +from the [`test-prof` gem](https://rubygems.org/gems/test-prof). + +``` +let_it_be(:project) { create(:project) } +let_it_be(:user) { create(:user) } + +before_all do + project.add_guest(user) +end +``` + +This will result in only one `Project`, `User`, and `ProjectMember` created for this context. + +`let_it_be` and `before_all` are also available within nested contexts. Cleanup after the context +is handled automatically using a transaction rollback. + +Note that if you modify an object defined inside a `let_it_be` block, +then you will need to reload the object as needed, or specify the `reload` +option to reload for every example. + +``` +let_it_be(:project, reload: true) { create(:project) } +``` + +You can also specify the `refind` option as well to completely load a +new object. + +``` +let_it_be(:project, refind: true) { create(:project) } +``` ### `set` variables +NOTE: **Note:** +We are incrementally removing `set` in favour of `let_it_be`. See the +[removal issue](https://gitlab.com/gitlab-org/gitlab/issues/27922). + In some cases there is no need to recreate the same object for tests again for each example. For example, a project is needed to test issues on the same project, one project will do for the entire file. This can be achieved by using @@ -308,7 +347,7 @@ them unspecified, and look up the value after the row is created. #### Redis -GitLab stores two main categories of data in Redis: cached items, and sidekiq +GitLab stores two main categories of data in Redis: cached items, and Sidekiq jobs. In most specs, the Rails cache is actually an in-memory store. This is replaced @@ -532,7 +571,7 @@ GitLab uses [factory_bot] as a test fixture replacement. - There should be only one top-level factory definition per file. - FactoryBot methods are mixed in to all RSpec groups. This means you can (and should) call `create(...)` instead of `FactoryBot.create(...)`. -- Make use of [traits] to clean up definitions and usages. +- Make use of [traits](https://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Traits) to clean up definitions and usages. - When defining a factory, don't define attributes that are not required for the resulting record to pass validation. - When instantiating from a factory, don't supply attributes that aren't @@ -541,7 +580,6 @@ GitLab uses [factory_bot] as a test fixture replacement. [See example](https://gitlab.com/gitlab-org/gitlab-foss/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). [factory_bot]: https://github.com/thoughtbot/factory_bot -[traits]: http://www.rubydoc.info/gems/factory_bot/file/GETTING_STARTED.md#Traits ### Fixtures @@ -549,9 +587,9 @@ All fixtures should be placed under `spec/fixtures/`. ### Repositories -Testing some functionality, e.g., merging a merge request, requires a git +Testing some functionality, e.g., merging a merge request, requires a Git repository with a certain state to be present in the test environment. GitLab -maintains the [gitlab-test](https://gitlab.com/gitlab-org/gitlab-test) +maintains the [`gitlab-test`](https://gitlab.com/gitlab-org/gitlab-test) repository for certain common cases - you can ensure a copy of the repository is used with the `:repository` trait for project factories: diff --git a/doc/development/testing_guide/ci.md b/doc/development/testing_guide/ci.md index d9f66a827de..5bdd0a69d7f 100644 --- a/doc/development/testing_guide/ci.md +++ b/doc/development/testing_guide/ci.md @@ -4,27 +4,24 @@ Our current CI parallelization setup is as follows: -1. The `knapsack` job in the prepare stage that is supposed to ensure we have a - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: - - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched - from S3, if it's not here we initialize the file with `{}`. -1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly - distributed share of tests: - - It works because the jobs have access to the - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts - from all previous stages are passed by default". +1. The `retrieve-tests-metadata` job in the `prepare` stage ensures we have a + `knapsack/report-master.json` file: + - The `knapsack/report-master.json` file is fetched from S3, if it's not here + we initialize the file with `{}`. +1. Each `[rspec|rspec-ee] [unit|integration|system|geo] n m` job are run with + `knapsack rspec` and should have an evenly distributed share of tests: + - It works because the jobs have access to the `knapsack/report-master.json` + since the "artifacts from all previous stages are passed by default". - the jobs set their own report path to - `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`. + `"knapsack/${TEST_TOOL}_${TEST_LEVEL}_${DATABASE}_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json"`. - if knapsack is doing its job, test files that are run should be listed under `Report specs`, not under `Leftover specs`. -1. The `update-knapsack` job takes all the - `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` - files from the `rspec x y` jobs and merge them all together into a single - `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then - uploaded to S3. - -After that, the next pipeline will use the up-to-date -`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. +1. The `update-tests-metadata` job (which only runs on scheduled pipelines for + [the canonical project](https://gitlab.com/gitlab-org/gitlab) takes all the + `knapsack/rspec*_pg_*.json` files and merge them all together into a single + `knapsack/report-master.json` file that is then uploaded to S3. + +After that, the next pipeline will use the up-to-date `knapsack/report-master.json` file. ## Monitoring diff --git a/doc/development/testing_guide/end_to_end/best_practices.md b/doc/development/testing_guide/end_to_end/best_practices.md index 2200069ecfd..042879b47aa 100644 --- a/doc/development/testing_guide/end_to_end/best_practices.md +++ b/doc/development/testing_guide/end_to_end/best_practices.md @@ -53,3 +53,15 @@ In summary: - **Do**: Split tests across separate files, unless the tests share expensive setup. - **Don't**: Put new tests in an existing file without considering the impact on parallelization. + +## Limit the use of `before(:all)` hook + +Limit the use of `before(:all)` to perform setup tasks with only API calls, non UI operations +or basic UI operations such as login. + +We use [`capybara-screenshot`](https://github.com/mattheworiordan/capybara-screenshot) library to automatically save screenshots on failures. +This library [saves the screenshots in the RSpec's `after` hook](https://github.com/mattheworiordan/capybara-screenshot/blob/master/lib/capybara-screenshot/rspec.rb#L97). +[If there is a failure in `before(:all)`, the `after` hook is not called](https://github.com/rspec/rspec-core/pull/2652/files#diff-5e04af96d5156e787f28d519a8c99615R148) and so the screenshots are not saved. + +Given this fact, we should limit the use of `before(:all)` to only those operations where a screenshot is not +necessary in case of failure and QA logs would be enough for debugging. diff --git a/doc/development/testing_guide/end_to_end/index.md b/doc/development/testing_guide/end_to_end/index.md index 9685a61d0c1..a9fb4be284e 100644 --- a/doc/development/testing_guide/end_to_end/index.md +++ b/doc/development/testing_guide/end_to_end/index.md @@ -16,14 +16,14 @@ a black-box testing framework for the API and the UI. ### Testing nightly builds We run scheduled pipeline each night to test nightly builds created by Omnibus. -You can find these nightly pipelines at [gitlab-org/quality/nightly/pipelines][quality-nightly-pipelines]. -Results are reported in the `#qa-nightly` Slack channel. +You can find these nightly pipelines at `https://gitlab.com/gitlab-org/quality/nightly/pipelines` +(need Developer access permissions). Results are reported in the `#qa-nightly` Slack channel. ### Testing staging We run scheduled pipeline each night to test staging. -You can find these nightly pipelines at [gitlab-org/quality/staging/pipelines][quality-staging-pipelines]. -Results are reported in the `#qa-staging` Slack channel. +You can find these nightly pipelines at `https://gitlab.com/gitlab-org/quality/staging/pipelines` +(need developer access permissions). Results are reported in the `#qa-staging` Slack channel. ### Testing code in merge requests @@ -52,7 +52,7 @@ graph LR A1 -.->|1. Triggers an omnibus-gitlab pipeline and wait for it to be done| A2 B2[`Trigger-qa` stage<br>`Trigger:qa-test` job] -.->|2. Triggers a gitlab-qa pipeline and wait for it to be done| A3 -subgraph "gitlab-ce/ee pipeline" +subgraph "gitlab-foss/gitlab pipeline" A1[`test` stage<br>`package-and-qa-manual` job] end @@ -135,20 +135,16 @@ Continued reading: You can ask question in the `#quality` channel on Slack (GitLab internal) or you can find an issue you would like to work on in -[the `gitlab-ce` issue tracker][gitlab-ce-issues], -[the `gitlab-ee` issue tracker][gitlab-ce-issues], or +[the `gitlab` issue tracker][gitlab-issues], or [the `gitlab-qa` issue tracker][gitlab-qa-issues]. [omnibus-gitlab]: https://gitlab.com/gitlab-org/omnibus-gitlab [gitlab-qa]: https://gitlab.com/gitlab-org/gitlab-qa [gitlab-qa-readme]: https://gitlab.com/gitlab-org/gitlab-qa/tree/master/README.md -[quality-nightly-pipelines]: https://gitlab.com/gitlab-org/quality/nightly/pipelines -[quality-staging-pipelines]: https://gitlab.com/gitlab-org/quality/staging/pipelines [review-apps]: ../review_apps.md [gitlab-qa-architecture]: https://gitlab.com/gitlab-org/gitlab-qa/blob/master/docs/architecture.md [gitlab-qa-issues]: https://gitlab.com/gitlab-org/gitlab-qa/issues?label_name%5B%5D=new+scenario -[gitlab-ce-issues]: https://gitlab.com/gitlab-org/gitlab-foss/issues?label_name[]=QA&label_name[]=test -[gitlab-ee-issues]: https://gitlab.com/gitlab-org/gitlab/issues?label_name[]=QA&label_name[]=test +[gitlab-issues]: https://gitlab.com/gitlab-org/gitlab/issues?label_name[]=QA&label_name[]=test [test environment orchestration scenarios]: https://gitlab.com/gitlab-org/gitlab-qa/tree/master/lib/gitlab/qa/scenario [instance-level scenarios]: https://gitlab.com/gitlab-org/gitlab-foss/tree/master/qa/qa/specs/features [Page objects documentation]: https://gitlab.com/gitlab-org/gitlab/tree/master/qa/qa/page/README.md diff --git a/doc/development/testing_guide/end_to_end/page_objects.md b/doc/development/testing_guide/end_to_end/page_objects.md index 8820b54fa87..28111c18378 100644 --- a/doc/development/testing_guide/end_to_end/page_objects.md +++ b/doc/development/testing_guide/end_to_end/page_objects.md @@ -2,7 +2,7 @@ In GitLab QA we are using a known pattern, called _Page Objects_. -This means that we have built an abstraction for all GitLab pages that we use +This means that we have built an abstraction for all pages in GitLab that we use to drive GitLab QA scenarios. Whenever we do something on a page, like filling in a form, or clicking a button, we do that only through a page object associated with this area of GitLab. diff --git a/doc/development/testing_guide/end_to_end/quick_start_guide.md b/doc/development/testing_guide/end_to_end/quick_start_guide.md index f5a46d574b0..2457d8ada5a 100644 --- a/doc/development/testing_guide/end_to_end/quick_start_guide.md +++ b/doc/development/testing_guide/end_to_end/quick_start_guide.md @@ -24,7 +24,7 @@ If you don't exactly understand what we mean by **not everything needs to happen ### 0. Are end-to-end tests needed? -At GitLab we respect the [test pyramid](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/development/testing_guide/testing_levels.md), and so, we recommend you check the code coverage of a specific feature before writing end-to-end tests, for both [CE](https://gitlab-org.gitlab.io/gitlab-ce/coverage-ruby/#_AllFiles) and [EE](https://gitlab-org.gitlab.io/gitlab-ee/coverage-ruby/#_AllFiles) projects. +At GitLab we respect the [test pyramid](https://gitlab.com/gitlab-org/gitlab/blob/master/doc/development/testing_guide/testing_levels.md), and so, we recommend you check the code coverage of a specific feature before writing end-to-end tests, for both [CE](https://gitlab-org.gitlab.io/gitlab-foss/coverage-ruby/#_AllFiles) and [EE](https://gitlab-org.gitlab.io/gitlab/coverage-ruby/#_AllFiles) projects. Sometimes you may notice that there is already good coverage in other test levels, and we can stay confident that if we break a feature, we will still have quick feedback about it, even without having end-to-end tests. @@ -38,7 +38,7 @@ The GitLab QA end-to-end tests are organized by the different [stages in the Dev > There may be sub-directories inside the stages directories, for different features. For example: `.../browser_ui/2_plan/ee_epics/` and `.../browser_ui/2_plan/issues/`. -Now, let's say we want to create tests for the [scoped labels](https://about.gitlab.com/2019/04/22/gitlab-11-10-released/#scoped-labels) feature, available on GitLab EE Premium (this feature is part of the Plan stage.) +Now, let's say we want to create tests for the [scoped labels](https://about.gitlab.com/blog/2019/04/22/gitlab-11-10-released/#scoped-labels) feature, available on GitLab EE Premium (this feature is part of the Plan stage.) > Because these tests are for a feature available only on GitLab EE, we need to create them in the [EE repository](https://gitlab.com/gitlab-org/gitlab). diff --git a/doc/development/testing_guide/flaky_tests.md b/doc/development/testing_guide/flaky_tests.md index 8e73b3d9ae9..0823c2e02b8 100644 --- a/doc/development/testing_guide/flaky_tests.md +++ b/doc/development/testing_guide/flaky_tests.md @@ -51,7 +51,7 @@ is detected in any other branch (`flaky-examples-check` job). In the future, the This was originally implemented in: <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/13021>. [rspec-retry]: https://github.com/NoRedInk/rspec-retry -[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/spec_helper.rb +[`spec/spec_helper.rb`]: https://gitlab.com/gitlab-org/gitlab/blob/master/spec/spec_helper.rb ## Problems we had in the past at GitLab @@ -61,7 +61,7 @@ This was originally implemented in: <https://gitlab.com/gitlab-org/gitlab-foss/m - [Capybara.reset_session! should be called before requests are blocked](https://gitlab.com/gitlab-org/gitlab-foss/issues/33779): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/12224> - FFaker generates funky data that tests are not ready to handle (and tests should be predictable so that's bad!): - [Make `spec/mailers/notify_spec.rb` more robust](https://gitlab.com/gitlab-org/gitlab-foss/issues/20121): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10015> - - [Transient failure in spec/requests/api/commits_spec.rb](https://gitlab.com/gitlab-org/gitlab-foss/issues/27988#note_25342521): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/9944> + - [Transient failure in `spec/requests/api/commits_spec.rb`](https://gitlab.com/gitlab-org/gitlab-foss/issues/27988#note_25342521): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/9944> - [Replace FFaker factory data with sequences](https://gitlab.com/gitlab-org/gitlab-foss/issues/29643): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10184> - [Transient failure in spec/finders/issues_finder_spec.rb](https://gitlab.com/gitlab-org/gitlab-foss/issues/30211#note_26707685): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10404> @@ -80,6 +80,9 @@ This was originally implemented in: <https://gitlab.com/gitlab-org/gitlab-foss/m - [Bis](https://gitlab.com/gitlab-org/gitlab-foss/issues/34609#note_34048715): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/12604> - [Bis](https://gitlab.com/gitlab-org/gitlab-foss/issues/34698#note_34276286): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/12664> - [Assert against the underlying database state instead of against a page's content](https://gitlab.com/gitlab-org/gitlab-foss/issues/31437): <https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/10934> +- In JS tests, shifting elements can cause Capybara to misclick when the element moves at the exact time Capybara sends the click + - [Dropdowns rendering upward or downward due to window size and scroll position](https://gitlab.com/gitlab-org/gitlab/merge_requests/17660) + - [Lazy loaded images can cause Capybara to misclick](https://gitlab.com/gitlab-org/gitlab/merge_requests/18713) #### Capybara viewport size related issues diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index da843218d8b..314995ca9b3 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -38,7 +38,7 @@ which could arise (especially with testing against browser specific features). - Jest runs in a Node.js environment, not in a browser. Support for running Jest tests in a browser [is planned](https://gitlab.com/gitlab-org/gitlab-foss/issues/58205). - Because Jest runs in a Node.js environment, it uses [jsdom](https://github.com/jsdom/jsdom) by default. See also its [limitations](#limitations-of-jsdom) below. - Jest does not have access to Webpack loaders or aliases. - The aliases used by Jest are defined in its [own config](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/jest.config.js). + The aliases used by Jest are defined in its [own config](https://gitlab.com/gitlab-org/gitlab/blob/master/jest.config.js). - All calls to `setTimeout` and `setInterval` are mocked away. See also [Jest Timer Mocks](https://jestjs.io/docs/en/timer-mocks). - `rewire` is not required because Jest supports mocking modules. See also [Manual Mocks](https://jestjs.io/docs/en/manual-mocks). - No [context object](https://jasmine.github.io/tutorials/your_first_suite#section-The_%3Ccode%3Ethis%3C/code%3E_keyword) is passed to tests in Jest. @@ -67,13 +67,13 @@ Running `yarn jest-debug` will run Jest in debug mode, allowing you to debug/ins ### Timeout error The default timeout for Jest is set in -[`/spec/frontend/test_setup.js`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/frontend/test_setup.js). +[`/spec/frontend/test_setup.js`](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/frontend/test_setup.js). If your test exceeds that time, it will fail. If you cannot improve the performance of the tests, you can increase the timeout for a specific test using -[`setTestTimeout`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/frontend/helpers/timeout.js). +[`setTestTimeout`](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/frontend/helpers/timeout.js). ```javascript import { setTestTimeout } from 'helpers/timeout'; @@ -388,7 +388,7 @@ it('renders something', done => { ##### `setTimeout()` / `setInterval()` in application If the application itself is waiting for some time, mock await the waiting. In Jest this is already -[done by default](https://gitlab.com/gitlab-org/gitlab-foss/blob/a2128edfee799e49a8732bfa235e2c5e14949c68/jest.config.js#L47) +[done by default](https://gitlab.com/gitlab-org/gitlab/blob/a2128edfee799e49a8732bfa235e2c5e14949c68/jest.config.js#L47) (see also [Jest Timer Mocks](https://jestjs.io/docs/en/timer-mocks)). In Karma you can use the [Jasmine mock clock](https://jasmine.github.io/api/2.9/Clock.html). @@ -482,7 +482,7 @@ As long as the fixtures don't change, `yarn test` is sufficient (and saves you s While developing locally, it may be helpful to keep Karma running so that you can get instant feedback on as you write tests and modify code. To do this -you can start Karma with `yarn run karma-start`. It will compile the javascript +you can start Karma with `yarn run karma-start`. It will compile the JavaScript assets and run a server at `http://localhost:9876/` where it will automatically run the tests on any browser which connects to it. You can enter that url on multiple browsers at once to have it run the tests on each in parallel. @@ -516,11 +516,6 @@ glob otherwise your shell may split it into multiple arguments: yarn karma -f 'spec/javascripts/ide/**/file_spec.js' ``` -## RSpec feature integration tests - -Information on setting up and running RSpec integration tests with -[Capybara] can be found in the [Testing Best Practices](best_practices.md). - ## Frontend test fixtures Code that is added to HAML templates (in `app/views/`) or makes Ajax requests to the backend has tests that require HTML or JSON from the backend. @@ -598,7 +593,6 @@ end [karma]: http://karma-runner.github.io/ [vue-test]: ../fe_guide/vue.md#testing-vue-components [rspec]: https://github.com/rspec/rspec-rails#feature-specs -[capybara]: https://github.com/teamcapybara/capybara [jasmine]: https://jasmine.github.io/ ## Overview of Frontend Testing Levels @@ -955,7 +949,11 @@ graph RL In contrast to [frontend integration tests](#frontend-integration-tests), feature tests make requests against the real backend instead of using fixtures. This also implies that database queries are executed which makes this category significantly slower. -See also the [RSpec testing guidelines](../testing_guide/best_practices.md#rspec). +See also + +- The [RSpec testing guidelines](../testing_guide/best_practices.md#rspec). +- System / Feature tests in the [Testing Best Practices](best_practices.md#system--feature-tests). +- [Issue #26159](https://gitlab.com/gitlab-org/gitlab/issues/26159) which aims at combine those guidelines with this page. ```mermaid graph RL @@ -1048,7 +1046,7 @@ testAction( ); ``` -Check an example in [spec/javascripts/ide/stores/actions_spec.jsspec/javascripts/ide/stores/actions_spec.js](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/spec/javascripts/ide/stores/actions_spec.js). +Check an example in [spec/javascripts/ide/stores/actions_spec.jsspec/javascripts/ide/stores/actions_spec.js](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/javascripts/ide/stores/actions_spec.js). ### Vue Helper: `mountComponent` diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index 173471e3af8..25da72b6b6a 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -19,7 +19,7 @@ integration testing. Following are two great articles that everyone should read to understand what automated testing means, and what are its principles: -- [Five Factor Testing](https://www.devmynd.com/blog/five-factor-testing): Why do we need tests? +- [Five Factor Testing](https://madeintandem.com/blog/five-factor-testing/): Why do we need tests? - [Principles of Automated Testing](http://www.lihaoyi.com/post/PrinciplesofAutomatedTesting.html): Levels of testing. Prioritize tests. Cost of tests. ## [Testing levels](testing_levels.md) @@ -60,6 +60,10 @@ Everything you should know about how to test Rake tasks. Everything you should know about how to run end-to-end tests using [GitLab QA][gitlab-qa] testing framework. +## [Migrations tests](testing_migrations_guide.md) + +Everything you should know about how to test migrations. + [Return to Development documentation](../README.md) [RSpec]: https://github.com/rspec/rspec-rails#feature-specs diff --git a/doc/development/testing_guide/review_apps.md b/doc/development/testing_guide/review_apps.md index 8ce25376a05..3dd403f148e 100644 --- a/doc/development/testing_guide/review_apps.md +++ b/doc/development/testing_guide/review_apps.md @@ -16,23 +16,23 @@ graph TD review-build-cng -->|once the `review-build-cng` job is done| review-deploy review-deploy -->|once the `review-deploy` job is done| review-qa-smoke -subgraph "1. gitlab-ce/ee `prepare` stage" +subgraph "1. gitlab-foss/gitlab `prepare` stage" build-qa-image end -subgraph "2. gitlab-ce/ee `test` stage" +subgraph "2. gitlab-foss/gitlab `test` stage" gitlab:assets:compile end -subgraph "3. gitlab-ce/ee `review-prepare` stage" +subgraph "3. gitlab-foss/gitlab `review-prepare` stage" review-build-cng end -subgraph "4. gitlab-ce/ee `review` stage" +subgraph "4. gitlab-foss/gitlab `review` stage" review-deploy["review-deploy<br><br>Helm deploys the Review App using the Cloud<br/>Native images built by the CNG-mirror pipeline.<br><br>Cloud Native images are deployed to the `review-apps-ce` or `review-apps-ee`<br>Kubernetes (GKE) cluster, in the GCP `gitlab-review-apps` project."] end -subgraph "5. gitlab-ce/ee `qa` stage" +subgraph "5. gitlab-foss/gitlab `qa` stage" review-qa-smoke[review-qa-smoke<br><br>gitlab-qa runs the smoke suite against the Review App.] end @@ -142,7 +142,7 @@ their node under pressure. ### Log into my Review App The default username is `root` and its password can be found in the 1Password -secure note named **gitlab-{ce,ee} Review App's root password**. +secure note named `gitlab-{ce,ee} Review App's root password`. ### Enable a feature flag for my Review App @@ -193,7 +193,7 @@ The following items may help diagnose this: - [Instance Group size in GCP](https://console.cloud.google.com/compute/instanceGroups/details/us-central1-b/gke-review-apps-ee-preemp-n1-standard-8affc0f5-grp?project=gitlab-review-apps&tab=monitoring&graph=GCE_SIZE&duration=P30D) - aids in identifying load spikes on the cluster. Kubernetes will add nodes up to 220 based on total resource requests. - `kubectl top nodes --sort-by=cpu` - can identify if node spikes are common or load on specific nodes which may get rebalanced by the Kubernetes scheduler. - `kubectl top pods --sort-by=cpu` - -- [K9s] - K9s is a powerful command line dashboard which allows you to filter by labels. This can help identify trends with apps exceeding the [review-app resource requests](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/review_apps/base-config.yaml). Kubernetes will schedule pods to nodes based on resource requests and allow for CPU usage up to the limits. +- [K9s] - K9s is a powerful command line dashboard which allows you to filter by labels. This can help identify trends with apps exceeding the [review-app resource requests](https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/review_apps/base-config.yaml). Kubernetes will schedule pods to nodes based on resource requests and allow for CPU usage up to the limits. - In K9s you can sort or add filters by typing the `/` character - `-lrelease=<review-app-slug>` - filters down to all pods for a release. This aids in determining what is having issues in a single deployment - `-lapp=<app>` - filters down to all pods for a specific app. This aids in determining resource usage by app. @@ -232,10 +232,10 @@ using `v232`. For the record, the debugging steps to find out this issue were: -1. Switch kubectl context to review-apps-ce (we recommend using [kubectx](https://kubectx.dev/)) +1. Switch kubectl context to review-apps-ce (we recommend using [kubectx](https://github.com/ahmetb/kubectx/)) 1. `kubectl get pods | grep dns` 1. `kubectl describe pod <pod name>` & confirm exact error message -1. Web search for exact error message, following rabbit hole to [a relevant kubernetes bug report](https://github.com/kubernetes/kubernetes/issues/57345) +1. Web search for exact error message, following rabbit hole to [a relevant Kubernetes bug report](https://github.com/kubernetes/kubernetes/issues/57345) 1. Access the node over SSH via the GCP console (**Computer Engine > VM instances** then click the "SSH" button for the node where the `dns-gitlab-review-app-external-dns` pod runs) 1. In the node: `systemctl --version` => systemd 232 @@ -311,8 +311,8 @@ find a way to limit it to only us.** [review-apps-ee]: https://console.cloud.google.com/kubernetes/clusters/details/us-central1-b/review-apps-ee?project=gitlab-review-apps [review-apps.sh]: https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/review_apps/review-apps.sh [automated_cleanup.rb]: https://gitlab.com/gitlab-org/gitlab/blob/master/scripts/review_apps/automated_cleanup.rb -[Auto-DevOps.gitlab-ci.yml]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml -[gitlab-ci-yml]: https://gitlab.com/gitlab-org/gitlab-foss/blob/master/.gitlab-ci.yml +[Auto-DevOps.gitlab-ci.yml]: https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +[gitlab-ci-yml]: https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab-ci.yml [gitlab-k8s-integration]: ../../user/project/clusters/index.md [K9s]: https://github.com/derailed/k9s [password-bug]: https://gitlab.com/gitlab-org/gitlab-foss/issues/53621 diff --git a/doc/development/testing_guide/testing_levels.md b/doc/development/testing_guide/testing_levels.md index b9436abf856..13659d66180 100644 --- a/doc/development/testing_guide/testing_levels.md +++ b/doc/development/testing_guide/testing_levels.md @@ -44,7 +44,7 @@ records should use stubs/doubles as much as possible. | `config/routes.rb`, `config/routes/` | `spec/routing/` | RSpec | | | `config/puma.example.development.rb`, `config/unicorn.rb.example` | `spec/rack_servers/` | RSpec | | | `db/` | `spec/db/` | RSpec | | -| `db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details at [`spec/migrations/README.md`](https://gitlab.com/gitlab-org/gitlab/blob/master/spec/migrations/README.md). | +| `db/{post_,}migrate/` | `spec/migrations/` | RSpec | More details in the [Testing Rails migrations guide](testing_migrations_guide.md). | | `Gemfile` | `spec/dependencies/`, `spec/sidekiq/` | RSpec | | | `lib/` | `spec/lib/` | RSpec | | | `lib/tasks/` | `spec/tasks/` | RSpec | | @@ -99,7 +99,7 @@ Formal definitions: - <https://en.wikipedia.org/wiki/White-box_testing> These kind of tests ensure the GitLab *Rails* application (i.e. -`gitlab-ce`/`gitlab-ee`) works as expected from a *browser* point of view. +`gitlab-foss`/`gitlab`) works as expected from a *browser* point of view. Note that: diff --git a/doc/development/testing_guide/testing_migrations_guide.md b/doc/development/testing_guide/testing_migrations_guide.md new file mode 100644 index 00000000000..b28d17a4b55 --- /dev/null +++ b/doc/development/testing_guide/testing_migrations_guide.md @@ -0,0 +1,216 @@ +--- +type: reference +--- + +# Testing Rails migrations at GitLab + +In order to reliably check Rails migrations, we need to test them against +a database schema. + +## When to write a migration test + +- Post migrations (`/db/post_migrate`) and background migrations + (`lib/gitlab/background_migration`) **must** have migration tests performed. +- If your migration is a data migration then it **must** have a migration test. +- Other migrations may have a migration test if necessary. + +## How does it work? + +Adding a `:migration` tag to a test signature enables some custom RSpec +`before` and `after` hooks in our +[`spec_helper.rb`](https://gitlab.com/gitlab-org/gitlab/blob/3b29908a64ff729c0cf6d93452fe00ab23079c75/spec%2Fspec_helper.rb#L259) +to run. + +A `before` hook will revert all migrations to the point that a migration +under test is not yet migrated. + +In other words, our custom RSpec hooks will find a previous migration, and +migrate the database **down** to the previous migration version. + +With this approach you can test a migration against a database schema. + +An `after` hook will migrate the database **up** and reinstitute the latest +schema version, so that the process does not affect subsequent specs and +ensures proper isolation. + +## Testing an `ActiveRecord::Migration` class + +To test an `ActiveRecord::Migration` class (i.e., a +regular migration `db/migrate` or a post-migration `db/post_migrate`), you +will need to manually `require` the migration file because it is not +autoloaded with Rails. Example: + +```ruby +require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb') +``` + +### Test helpers + +#### `table` + +Use the `table` helper to create a temporary `ActiveRecord::Base`-derived model +for a table. [FactoryBot](https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories) +**should not** be used to create data for migration specs. For example, to +create a record in the `projects` table: + +```ruby +project = table(:projects).create!(id: 1, name: 'gitlab1', path: 'gitlab1') +``` + +#### `migrate!` + +Use the `migrate!` helper to run the migration that is under test. It will not only +run the migration, but will also bump the schema version in the `schema_migrations` +table. It is necessary because in the `after` hook we trigger the rest of +the migrations, and we need to know where to start. Example: + +```ruby +it 'migrates successfully' do + # ... pre-migration expectations + + migrate! + + # ... post-migration expectations +end +``` + +#### `reversible_migration` + +Use the `reversible_migration` helper to test migrations with either a +`change` or both `up` and `down` hooks. This will test that the state of +the application and its data after the migration becomes reversed is the +same as it was before the migration ran in the first place. The helper: + +1. Runs the `before` expectations before the **up** migration. +1. Migrates **up**. +1. Runs the `after` expectations. +1. Migrates **down**. +1. Runs the `before` expectations a second time. + +Example: + +```ruby +reversible_migration do |migration| + migration.before -> { + # ... pre-migration expectations + } + + migration.after -> { + # ... post-migration expectations + } +end +``` + +### Example database migration test + +This spec tests the +[`db/post_migrate/20170526185842_migrate_pipeline_stages.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/db/post_migrate/20170526185842_migrate_pipeline_stages.rb) +migration. You can find the complete spec in +[`spec/migrations/migrate_pipeline_stages_spec.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/spec/migrations/migrate_pipeline_stages_spec.rb). + +```ruby +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170526185842_migrate_pipeline_stages.rb') + +describe MigratePipelineStages, :migration do + # Create test data - pipeline and CI/CD jobs. + let(:jobs) { table(:ci_builds) } + let(:stages) { table(:ci_stages) } + let(:pipelines) { table(:ci_pipelines) } + let(:projects) { table(:projects) } + + before do + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a') + jobs.create!(id: 1, commit_id: 1, project_id: 123, stage_idx: 2, stage: 'build') + jobs.create!(id: 2, commit_id: 1, project_id: 123, stage_idx: 1, stage: 'test') + end + + # Test just the up migration. + it 'correctly migrates pipeline stages' do + expect(stages.count).to be_zero + + migrate! + + expect(stages.count).to eq 2 + expect(stages.all.pluck(:name)).to match_array %w[test build] + end + + # Test a reversible migration. + it 'correctly migrates up and down pipeline stages' do + reversible_migration do |migration| + # Expectations will run before the up migration, + # and then again after the down migration + migration.before -> { + expect(stages.count).to be_zero + } + + # Expectations will run after the up migration. + migration.after -> { + expect(stages.count).to eq 2 + expect(stages.all.pluck(:name)).to match_array %w[test build] + } + end +end +``` + +## Testing a non-`ActiveRecord::Migration` class + +To test a non-`ActiveRecord::Migration` test (a background migration), +you will need to manually provide a required schema version. Please add a +schema tag to a context that you want to switch the database schema within. + +Example: + +```ruby +describe SomeClass, :migration, schema: 20170608152748 do + # ... +end +``` + +### Example background migration test + +This spec tests the +[`lib/gitlab/background_migration/archive_legacy_traces.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/lib/gitlab/background_migration/archive_legacy_traces.rb) +background migration. You can find the complete spec on +[`spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb`](https://gitlab.com/gitlab-org/gitlab/blob/v11.6.5/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb) + +```ruby +require 'spec_helper' + +describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do + include TraceHelpers + + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:builds) { table(:ci_builds) } + let(:job_artifacts) { table(:ci_job_artifacts) } + + before do + namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) + @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build') + end + + context 'when trace file exists at the right place' do + before do + create_legacy_trace(@build, 'trace in file') + end + + it 'correctly archive legacy traces' do + expect(job_artifacts.count).to eq(0) + expect(File.exist?(legacy_trace_path(@build))).to be_truthy + + described_class.new.perform(1, 1) + + expect(job_artifacts.count).to eq(1) + expect(File.exist?(legacy_trace_path(@build))).to be_falsy + expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file') + end + end +end +``` + +NOTE: **Note:** +These tests do not run within a database transaction, as we use a deletion database +cleanup strategy. Do not depend on a transaction being present. diff --git a/doc/development/uploads.md b/doc/development/uploads.md index 1af3e9db954..e3ff62f1d2f 100644 --- a/doc/development/uploads.md +++ b/doc/development/uploads.md @@ -82,7 +82,7 @@ To address this problem an HA object storage can be used and it's supported by [ Scaling NFS is outside of our support scope, and NFS is not a part of cloud native installations. -All features that require sidekiq and do not use object storage acceleration won't work without NFS. In Kubernetes, machine boundaries translate to PODs, and in this case the uploaded file will be written into the POD private disk. Since sidekiq POD cannot reach into other pods, the operation will fail to read it. +All features that require Sidekiq and do not use object storage acceleration won't work without NFS. In Kubernetes, machine boundaries translate to PODs, and in this case the uploaded file will be written into the POD private disk. Since Sidekiq POD cannot reach into other pods, the operation will fail to read it. ## How to select the proper level of acceleration? @@ -92,19 +92,19 @@ We can identify three major use-cases for an upload: 1. **storage:** if we are uploading for storing a file (i.e. artifacts, packages, discussion attachments). In this case [object storage acceleration](#workhorse-object-storage-acceleration) is the proper level as it's the less resource-intensive operation. Additional information can be found on [File Storage in GitLab](file_storage.md). 1. **in-controller/synchronous processing:** if we allow processing **small files** synchronously, using [disk acceleration](#workhorse-disk-acceleration) may speed up development. -1. **sidekiq/asynchronous processing:** Async processing must implement [object storage acceleration](#workhorse-object-storage-acceleration), the reason being that it's the only way to support Cloud Native deployments without a shared NFS. +1. **Sidekiq/asynchronous processing:** Async processing must implement [object storage acceleration](#workhorse-object-storage-acceleration), the reason being that it's the only way to support Cloud Native deployments without a shared NFS. For more details about currently broken feature see [epic &1802](https://gitlab.com/groups/gitlab-org/-/epics/1802). ### Handling repository uploads -Some features involves git repository uploads without using a regular git client. +Some features involves Git repository uploads without using a regular Git client. Some examples are uploading a repository file from the web interface and [design management](../user/project/issues/design_management.md). -Those uploads requires the rails controller to act as a git client in lieu of the user. +Those uploads requires the rails controller to act as a Git client in lieu of the user. Those operation falls into _in-controller/synchronous processing_ category, but we have no warranties on the file size. -In case of a LFS upload, the file pointer is committed synchronously, but file upload to object storage is performed asynchronously with sidekiq. +In case of a LFS upload, the file pointer is committed synchronously, but file upload to object storage is performed asynchronously with Sidekiq. ## Upload encodings @@ -114,7 +114,7 @@ We have three kinds of file encoding in our uploads: 1. <i class="fa fa-check-circle"></i> **multipart**: `multipart/form-data` is the most common, a file is encoded as a part of a multipart encoded request. 1. <i class="fa fa-check-circle"></i> **body**: some APIs uploads files as the whole request body. -1. <i class="fa fa-times-circle"></i> **JSON**: some JSON API uploads files as base64 encoded strings. This requires [gitlab-workhorse#226](https://gitlab.com/gitlab-org/gitlab-workhorse/issues/226) to be implemented. +1. <i class="fa fa-times-circle"></i> **JSON**: some JSON API uploads files as base64 encoded strings. This will require a change to GitLab Workhorse, which [is planned](https://gitlab.com/gitlab-org/gitlab-workhorse/issues/226). ## Uploading technologies @@ -209,10 +209,12 @@ This is the more advanced acceleration technique we have in place. Workhorse asks rails for temporary pre-signed object storage URLs and directly uploads to object storage. In this setup an extra rails route needs to be implemented in order to handle authorization, -you can see an example of this in [`Projects::LfsStorageController`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/app/controllers/projects/lfs_storage_controller.rb) -and [its routes](https://gitlab.com/gitlab-org/gitlab-foss/blob/v12.2.0/config/routes/git_http.rb#L31-32). +you can see an example of this in [`Projects::LfsStorageController`](https://gitlab.com/gitlab-org/gitlab/blob/cc723071ad337573e0360a879cbf99bc4fb7adb9/app/controllers/projects/lfs_storage_controller.rb) +and [its routes](https://gitlab.com/gitlab-org/gitlab/blob/cc723071ad337573e0360a879cbf99bc4fb7adb9/config/routes/git_http.rb#L31-32). -**note:** this will fallback to _Workhorse disk acceleration_ when object storage is not enabled in the gitlab instance. The answer to the `/authorize` call will only contain a file system path. +NOTE: **Note:** +This will fall back to _Workhorse disk acceleration_ when object storage is not enabled +in the GitLab instance. The answer to the `/authorize` call will only contain a file system path. ```mermaid sequenceDiagram @@ -267,4 +269,4 @@ sequenceDiagram This option affect the response to the `/authorize` call. When not enabled, the API response will not contain presigned URLs and workhorse will write the file the shared disk, on the path is provided by rails, acting like object storage was disabled. -Once the request reachs rails, it will schedule an object storage upload as a sidekiq job. +Once the request reachs rails, it will schedule an object storage upload as a Sidekiq job. diff --git a/doc/development/utilities.md b/doc/development/utilities.md index 11de0d56ef3..38e416d68e4 100644 --- a/doc/development/utilities.md +++ b/doc/development/utilities.md @@ -2,7 +2,9 @@ We developed a number of utilities to ease development. -## [`MergeHash`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/utils/merge_hash.rb) +## `MergeHash` + +Refer to: <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/merge_hash.rb>: - Deep merges an array of hashes: @@ -45,7 +47,9 @@ We developed a number of utilities to ease development. [:hello, "world", :this, :crushes, "an entire", "hash"] ``` -## [`Override`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/utils/override.rb) +## `Override` + +Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/override.rb>: - This utility could help us check if a particular method would override another method or not. It has the same idea of Java's `@Override` annotation @@ -90,7 +94,9 @@ We developed a number of utilities to ease development. end ``` -## [`StrongMemoize`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/utils/strong_memoize.rb) +## `StrongMemoize` + +Refer to <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/utils/strong_memoize.rb>: - Memoize the value even if it is `nil` or `false`. @@ -136,7 +142,9 @@ We developed a number of utilities to ease development. Find.new.clear_memoization(:result) ``` -## [`RequestCache`](https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/cache/request_cache.rb) +## `RequestCache` + +Refer to <https://gitlab.com/gitlab-org/gitlab/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, |
