From 859a6fb938bb9ee2a317c46dfa4fcc1af49608f0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 18 Feb 2021 10:34:06 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-9-stable-ee --- doc/development/README.md | 5 +- doc/development/adding_service_component.md | 2 +- doc/development/agent/gitops.md | 2 +- doc/development/agent/identity.md | 2 +- doc/development/agent/index.md | 2 +- doc/development/agent/local.md | 2 +- doc/development/agent/repository_overview.md | 98 +++ doc/development/agent/routing.md | 179 +++-- doc/development/agent/user_stories.md | 2 +- doc/development/api_graphql_styleguide.md | 91 ++- doc/development/api_styleguide.md | 2 +- doc/development/application_limits.md | 4 +- doc/development/approval_rules.md | 12 +- doc/development/architecture.md | 86 +- doc/development/auto_devops.md | 2 +- doc/development/changelog.md | 17 +- doc/development/chaos_endpoints.md | 59 ++ doc/development/chatops_on_gitlabcom.md | 16 +- doc/development/cicd/img/ci_minutes.png | Bin 0 -> 20223 bytes doc/development/cicd/index.md | 39 + doc/development/code_intelligence/index.md | 4 +- doc/development/code_review.md | 35 +- doc/development/contributing/index.md | 12 +- .../contributing/merge_request_workflow.md | 10 +- doc/development/contributing/style_guides.md | 99 ++- doc/development/cycle_analytics.md | 8 - doc/development/dangerbot.md | 4 +- .../database/database_reviewer_guidelines.md | 6 +- .../database/strings_and_the_text_data_type.md | 6 + doc/development/database/table_partitioning.md | 2 +- doc/development/database_debugging.md | 6 +- doc/development/database_review.md | 11 +- doc/development/diffs.md | 22 +- doc/development/directory_structure.md | 36 + doc/development/distributed_tracing.md | 2 +- doc/development/doc_styleguide.md | 8 - .../documentation/feature-change-workflow.md | 8 - doc/development/documentation/feature_flags.md | 28 +- .../documentation/improvement-workflow.md | 8 - doc/development/documentation/index.md | 201 +++-- .../documentation/restful_api_styleguide.md | 2 +- .../documentation/site_architecture/global_nav.md | 6 +- .../documentation/site_architecture/index.md | 2 +- doc/development/documentation/structure.md | 230 +++--- doc/development/documentation/styleguide.md | 3 + .../documentation/styleguide/img/tier_badge.png | Bin 9592 -> 9320 bytes doc/development/documentation/styleguide/index.md | 205 ++--- doc/development/documentation/testing.md | 58 +- doc/development/documentation/workflow.md | 6 +- doc/development/ee_features.md | 38 +- doc/development/elasticsearch.md | 14 +- doc/development/experiment_guide/index.md | 18 +- doc/development/export_csv.md | 2 +- doc/development/fe_guide/accessibility.md | 8 +- doc/development/fe_guide/architecture.md | 4 +- doc/development/fe_guide/axios.md | 2 +- doc/development/fe_guide/components.md | 3 - doc/development/fe_guide/dark_mode.md | 77 ++ doc/development/fe_guide/dependencies.md | 67 +- doc/development/fe_guide/design_anti_patterns.md | 219 +++++ doc/development/fe_guide/design_patterns.md | 78 +- doc/development/fe_guide/development_process.md | 6 +- doc/development/fe_guide/dropdowns.md | 3 - doc/development/fe_guide/editor_lite.md | 20 +- doc/development/fe_guide/emojis.md | 2 +- doc/development/fe_guide/frontend_faq.md | 39 +- doc/development/fe_guide/graphql.md | 167 ++-- doc/development/fe_guide/icons.md | 12 +- doc/development/fe_guide/index.md | 19 +- doc/development/fe_guide/performance.md | 48 +- doc/development/fe_guide/security.md | 4 +- doc/development/fe_guide/style/html.md | 32 + doc/development/fe_guide/style/javascript.md | 2 +- doc/development/fe_guide/style/scss.md | 20 +- doc/development/fe_guide/style_guide_js.md | 8 - doc/development/fe_guide/style_guide_scss.md | 8 - doc/development/fe_guide/testing.md | 8 - doc/development/fe_guide/tooling.md | 6 +- doc/development/fe_guide/troubleshooting.md | 41 + doc/development/fe_guide/vue.md | 40 +- doc/development/fe_guide/vue3_migration.md | 10 +- doc/development/fe_guide/vuex.md | 6 +- doc/development/feature_flags.md | 8 - doc/development/feature_flags/controls.md | 36 +- doc/development/feature_flags/development.md | 46 ++ doc/development/feature_flags/index.md | 3 +- doc/development/feature_flags/process.md | 6 +- doc/development/filtering_by_label.md | 4 +- doc/development/foreign_keys.md | 2 +- doc/development/frontend.md | 8 - doc/development/gemfile.md | 13 + doc/development/geo.md | 4 +- doc/development/geo/framework.md | 33 +- doc/development/gitaly.md | 43 +- doc/development/go_guide/dependencies.md | 2 +- doc/development/go_guide/index.md | 29 +- doc/development/graphql_guide/batchloader.md | 2 +- doc/development/graphql_guide/graphql_pro.md | 21 + doc/development/graphql_guide/index.md | 7 +- doc/development/graphql_guide/pagination.md | 4 +- doc/development/i18n/merging_translations.md | 20 +- doc/development/i18n_guide.md | 8 - doc/development/import_project.md | 2 +- doc/development/instrumentation.md | 2 +- doc/development/integrations/codesandbox.md | 34 +- doc/development/integrations/jenkins.md | 4 +- doc/development/integrations/jira_connect.md | 6 +- doc/development/integrations/secure.md | 2 + .../integrations/secure_partner_integration.md | 6 +- doc/development/interacting_components.md | 2 +- doc/development/internal_api.md | 172 +++- doc/development/iterating_tables_in_batches.md | 2 +- doc/development/kubernetes.md | 2 +- doc/development/lfs.md | 18 +- doc/development/licensed_feature_availability.md | 4 +- doc/development/licensing.md | 2 +- doc/development/logging.md | 26 +- doc/development/maintenance_mode.md | 19 + doc/development/module_with_instance_variables.md | 66 +- doc/development/new_fe_guide/dependencies.md | 38 +- .../new_fe_guide/development/testing.md | 8 - doc/development/new_fe_guide/index.md | 4 - .../new_fe_guide/modules/widget_extensions.md | 14 +- doc/development/new_fe_guide/style/html.md | 8 - doc/development/new_fe_guide/style/index.md | 8 - doc/development/new_fe_guide/style/javascript.md | 8 - doc/development/new_fe_guide/style/prettier.md | 8 - doc/development/ordering_table_columns.md | 22 +- doc/development/packages.md | 18 +- doc/development/performance.md | 101 ++- doc/development/pipelines.md | 66 +- doc/development/polling.md | 2 +- doc/development/product_analytics/index.md | 5 - doc/development/product_analytics/snowplow.md | 2 +- doc/development/product_analytics/usage_ping.md | 2 +- doc/development/profiling.md | 4 +- doc/development/projections.md | 4 +- doc/development/prometheus_metrics.md | 7 +- doc/development/pry_debugging.md | 7 +- doc/development/python_guide/index.md | 8 +- doc/development/rake_tasks.md | 46 +- doc/development/reactive_caching.md | 4 +- doc/development/redis.md | 11 +- doc/development/refactoring_guide/index.md | 2 +- doc/development/repository_mirroring.md | 6 +- .../rolling_out_changes_using_feature_flags.md | 8 - doc/development/secure_coding_guidelines.md | 70 +- doc/development/shared_files.md | 2 +- doc/development/shell_commands.md | 2 +- doc/development/sidekiq_debugging.md | 8 - doc/development/sidekiq_style_guide.md | 2 +- doc/development/snowplow.md | 312 ++++---- doc/development/stage_group_dashboards.md | 8 +- doc/development/telemetry/event_dictionary.md | 8 - doc/development/telemetry/index.md | 8 - doc/development/telemetry/snowplow.md | 8 - doc/development/telemetry/usage_ping.md | 8 - doc/development/testing.md | 8 - doc/development/testing_guide/best_practices.md | 151 ++-- .../testing_guide/end_to_end/best_practices.md | 4 +- .../testing_guide/end_to_end/feature_flags.md | 13 + .../testing_guide/end_to_end/page_objects.md | 2 +- .../end_to_end/rspec_metadata_tests.md | 33 +- .../running_tests_that_require_special_setup.md | 32 +- doc/development/testing_guide/frontend_testing.md | 70 +- doc/development/testing_guide/index.md | 4 +- doc/development/testing_guide/review_apps.md | 8 +- .../testing_guide/testing_migrations_guide.md | 2 +- doc/development/transient/prevention-patterns.md | 141 ++++ doc/development/ui_guide.md | 5 - doc/development/usage_ping.md | 129 +-- doc/development/usage_ping/dictionary.md | 887 +++++++++++++++++++++ doc/development/usage_ping/metrics_dictionary.md | 60 +- doc/development/utilities.md | 45 +- doc/development/ux_guide/animation.md | 5 - doc/development/ux_guide/basics.md | 5 - doc/development/ux_guide/components.md | 5 - doc/development/ux_guide/copy.md | 5 - doc/development/ux_guide/features.md | 5 - doc/development/ux_guide/illustrations.md | 5 - doc/development/ux_guide/index.md | 5 - doc/development/ux_guide/principles.md | 5 - doc/development/ux_guide/resources.md | 5 - doc/development/ux_guide/surfaces.md | 5 - doc/development/ux_guide/tips.md | 5 - doc/development/ux_guide/users.md | 8 - doc/development/wikis.md | 8 +- doc/development/windows.md | 4 +- doc/development/writing_documentation.md | 3 - 189 files changed, 4007 insertions(+), 1871 deletions(-) create mode 100644 doc/development/agent/repository_overview.md create mode 100644 doc/development/cicd/img/ci_minutes.png delete mode 100644 doc/development/cycle_analytics.md create mode 100644 doc/development/directory_structure.md delete mode 100644 doc/development/doc_styleguide.md delete mode 100644 doc/development/documentation/feature-change-workflow.md delete mode 100644 doc/development/documentation/improvement-workflow.md delete mode 100644 doc/development/fe_guide/components.md create mode 100644 doc/development/fe_guide/dark_mode.md create mode 100644 doc/development/fe_guide/design_anti_patterns.md delete mode 100644 doc/development/fe_guide/dropdowns.md delete mode 100644 doc/development/fe_guide/style_guide_js.md delete mode 100644 doc/development/fe_guide/style_guide_scss.md delete mode 100644 doc/development/fe_guide/testing.md create mode 100644 doc/development/fe_guide/troubleshooting.md delete mode 100644 doc/development/feature_flags.md delete mode 100644 doc/development/frontend.md create mode 100644 doc/development/graphql_guide/graphql_pro.md delete mode 100644 doc/development/i18n_guide.md create mode 100644 doc/development/maintenance_mode.md delete mode 100644 doc/development/new_fe_guide/development/testing.md delete mode 100644 doc/development/new_fe_guide/style/html.md delete mode 100644 doc/development/new_fe_guide/style/index.md delete mode 100644 doc/development/new_fe_guide/style/javascript.md delete mode 100644 doc/development/new_fe_guide/style/prettier.md delete mode 100644 doc/development/rolling_out_changes_using_feature_flags.md delete mode 100644 doc/development/sidekiq_debugging.md delete mode 100644 doc/development/telemetry/event_dictionary.md delete mode 100644 doc/development/telemetry/index.md delete mode 100644 doc/development/telemetry/snowplow.md delete mode 100644 doc/development/telemetry/usage_ping.md delete mode 100644 doc/development/testing.md create mode 100644 doc/development/transient/prevention-patterns.md delete mode 100644 doc/development/ui_guide.md create mode 100644 doc/development/usage_ping/dictionary.md delete mode 100644 doc/development/ux_guide/animation.md delete mode 100644 doc/development/ux_guide/basics.md delete mode 100644 doc/development/ux_guide/components.md delete mode 100644 doc/development/ux_guide/copy.md delete mode 100644 doc/development/ux_guide/features.md delete mode 100644 doc/development/ux_guide/illustrations.md delete mode 100644 doc/development/ux_guide/index.md delete mode 100644 doc/development/ux_guide/principles.md delete mode 100644 doc/development/ux_guide/resources.md delete mode 100644 doc/development/ux_guide/surfaces.md delete mode 100644 doc/development/ux_guide/tips.md delete mode 100644 doc/development/ux_guide/users.md delete mode 100644 doc/development/writing_documentation.md (limited to 'doc/development') diff --git a/doc/development/README.md b/doc/development/README.md index 0d3c1b3cbe9..3d5335feb11 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -121,6 +121,7 @@ In these cases, use the following workflow: - [User Experience (UX)](https://about.gitlab.com/handbook/engineering/ux/) - [Security](https://about.gitlab.com/handbook/engineering/security/) - [Quality](https://about.gitlab.com/handbook/engineering/quality/) + - [Engineering Productivity](https://about.gitlab.com/handbook/engineering/quality/engineering-productivity-team/) - [Infrastructure](https://about.gitlab.com/handbook/engineering/infrastructure/) - [Technical Writing](https://about.gitlab.com/handbook/engineering/ux/technical-writing/) @@ -151,6 +152,7 @@ In these cases, use the following workflow: ## Backend guides +- [Directory structure](directory_structure.md) - [GitLab utilities](utilities.md) - [Issuable-like Rails models](issuable-like-models.md) - [Logging](logging.md) @@ -229,7 +231,7 @@ See [database guidelines](database/index.md). - [Security Scanners](integrations/secure.md) - [Secure Partner Integration](integrations/secure_partner_integration.md) - [How to run Jenkins in development environment](integrations/jenkins.md) -- [How to run local Codesandbox integration for Web IDE Live Preview](integrations/codesandbox.md) +- [How to run local `Codesandbox` integration for Web IDE Live Preview](integrations/codesandbox.md) ## Testing guides @@ -293,6 +295,7 @@ See [database guidelines](database/index.md). - [Compatibility with multiple versions of the application running at the same time](multi_version_compatibility.md) - [Features inside `.gitlab/`](features_inside_dot_gitlab.md) - [Dashboards for stage groups](stage_group_dashboards.md) +- [Preventing transient bugs](transient/prevention-patterns.md) ## Other GitLab Development Kit (GDK) guides diff --git a/doc/development/adding_service_component.md b/doc/development/adding_service_component.md index 7e2add2c91f..f3e23906ac6 100644 --- a/doc/development/adding_service_component.md +++ b/doc/development/adding_service_component.md @@ -65,7 +65,7 @@ Notify the [Distribution team](https://about.gitlab.com/handbook/engineering/dev New services to be bundled with GitLab need to be available in the following environments. -**Dev environment** +**Development environment** The first step of bundling a new service is to provide it in the development environment to engage in collaboration and feedback. diff --git a/doc/development/agent/gitops.md b/doc/development/agent/gitops.md index 8c8586326fa..f183ba86aa1 100644 --- a/doc/development/agent/gitops.md +++ b/doc/development/agent/gitops.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# GitOps with the Kubernetes Agent **(PREMIUM ONLY)** +# GitOps with the Kubernetes Agent **(PREMIUM SELF)** The [GitLab Kubernetes Agent](../../user/clusters/agent/index.md) supports the [pull-based version](https://www.gitops.tech/#pull-based-deployments) of diff --git a/doc/development/agent/identity.md b/doc/development/agent/identity.md index 884ce015a02..49d20d2fd87 100644 --- a/doc/development/agent/identity.md +++ b/doc/development/agent/identity.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Kubernetes Agent identity and authentication **(PREMIUM ONLY)** +# Kubernetes Agent identity and authentication **(PREMIUM SELF)** This page uses the word `agent` to describe the concept of the GitLab Kubernetes Agent. The program that implements the concept is called `agentk`. diff --git a/doc/development/agent/index.md b/doc/development/agent/index.md index 95661c8ddbd..112162f8f90 100644 --- a/doc/development/agent/index.md +++ b/doc/development/agent/index.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Kubernetes Agent development **(PREMIUM ONLY)** +# Kubernetes Agent development **(PREMIUM SELF)** This page contains developer-specific information about the GitLab Kubernetes Agent. [End-user documentation about the GitLab Kubernetes Agent](../../user/clusters/agent/index.md) diff --git a/doc/development/agent/local.md b/doc/development/agent/local.md index 47246a6a6d3..603364567bf 100644 --- a/doc/development/agent/local.md +++ b/doc/development/agent/local.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Run the Kubernetes Agent locally **(PREMIUM ONLY)** +# Run the Kubernetes Agent locally **(PREMIUM SELF)** You can run `kas` and `agentk` locally to test the [Kubernetes Agent](index.md) yourself. diff --git a/doc/development/agent/repository_overview.md b/doc/development/agent/repository_overview.md new file mode 100644 index 00000000000..b9eea286a3e --- /dev/null +++ b/doc/development/agent/repository_overview.md @@ -0,0 +1,98 @@ +--- +stage: Configure +group: Configure +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +--- + +# Kubernetes Agent repository overview **(PREMIUM SELF)** + +This page describes the subfolders of the Kubernetes Agent repository. +[Development information](index.md) and +[end-user documentation](../../user/clusters/agent/index.md) are both available. + + +For a video overview, see +[GitLab Kubernetes Agent repository overview](https://www.youtube.com/watch?v=j8CyaCWroUY). + +## `build` + +Various files for the build process. + +### `build/deployment` + +A [`kpt`](https://googlecontainertools.github.io/kpt/) package that bundles some +[Kustomize](https://kustomize.io/) layers and components. Can be used as-is, or +to create a custom package to install `agentk`. + +## `cmd` + +Commands are binaries that this repository produces. They are: + +- `kas` is the GitLab Kubernetes Agent Server binary. +- `agentk` is the GitLab Kubernetes Agent binary. + +Each of these directories contain application bootstrap code for: + +- Reading configuration. +- Applying defaults to it. +- Constructing the dependency graph of objects that constitute the program. +- Running it. + +### `cmd/agentk` + +- `agentk` initialization logic. +- Implementation of the agent modules API. + +### `cmd/kas` + +- `kas` initialization logic. +- Implementation of the server modules API. + +## `examples` + +Git submodules for the example projects. + +## `internal` + +The main code of both `gitlab-kas` and `agentk`, and various supporting building blocks. + +### `internal/api` + +Structs that represent some important pieces of data. + +### `internal/gitaly` + +Items to work with [Gitaly](../../administration/gitaly/index.md). + +### `internal/gitlab` + +GitLab REST client. + +### `internal/module` + +Modules that implement server and agent-side functionality. + +### `internal/tool` + +Various building blocks. `internal/tool/testing` contains mocks and helpers +for testing. Mocks are generated with [`gomock`](https://pkg.go.dev/github.com/golang/mock). + +## `it` + +Contains scaffolding for integration tests. Unused at the moment. + +## `pkg` + +Contains exported packages. + +### `pkg/agentcfg` + +Contains protobuf definitions of the `agentk` configuration file. Used to configure +the agent through a configuration repository. + +### `pkg/kascfg` + +Contains protobuf definitions of the `gitlab-kas` configuration file. Contains an +example of that configuration file along with the test for it. The test ensures +the configuration file example is in sync with the protobuf definitions of the +file and defaults, which are applied when the file is loaded. diff --git a/doc/development/agent/routing.md b/doc/development/agent/routing.md index 43cc78ccdfb..9a7d6422d47 100644 --- a/doc/development/agent/routing.md +++ b/doc/development/agent/routing.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Routing `kas` requests in the Kubernetes Agent **(PREMIUM ONLY)** +# Routing `kas` requests in the Kubernetes Agent **(PREMIUM SELF)** This document describes how `kas` routes requests to concrete `agentk` instances. GitLab must talk to GitLab Kubernetes Agent Server (`kas`) to: @@ -134,90 +134,97 @@ of the `kas` receiving the request from the _external_ endpoint to retry and re- requests. This method ensures a single central component for each request can determine how a request is routed, rather than distributing the decision across several `kas` instances. -### API definitions +### Reverse gRPC tunnel + +This section explains how the `agentk` -> `kas` reverse gRPC tunnel is implemented. + + +For a video overview of how some of the blocks map to code, see +[GitLab Kubernetes Agent reverse gRPC tunnel architecture and code overview +](https://www.youtube.com/watch?v=9pnQF76hyZc). + +#### High level schema + +In this example, `Server side of module A` exposes its API to get the `Pod` list +on the `Public API gRPC server`. When it receives a request, it must determine +the agent ID from it, then call the proxying code which forwards the request to +a suitable `agentk` that can handle it. + +The `Agent side of module A` exposes the same API on the `Internal gRPC server`. +When it receives the request, it needs to handle it (such as retrieving and returning +the `Pod` list). + +This schema describes how reverse tunneling is handled fully transparently +for modules, so you can add new features: + +```mermaid +graph TB + subgraph kas + server-internal-grpc-server[Internal gRPC server] + server-api-grpc-server[Public API gRPC server] + server-module-a[Server side of module A] + server-module-b[Server side of module B] + end + subgraph agentk + agent-internal-grpc-server[Internal gRPC server] + agent-module-a[Agent side of module A] + agent-module-b[Agent side of module B] + end + + agent-internal-grpc-server -- request --> agent-module-a + agent-internal-grpc-server -- request --> agent-module-b + + server-module-a-. expose API on .-> server-internal-grpc-server + server-module-b-. expose API on .-> server-api-grpc-server + + server-internal-grpc-server -- proxy request --> agent-internal-grpc-server + server-api-grpc-server -- proxy request --> agent-internal-grpc-server +``` + +#### Implementation schema + +`HandleTunnelConnection()` is called with the server-side interface of the reverse +tunnel. It registers the connection and blocks, waiting for a request to proxy +through the connection. + +`HandleIncomingConnection()` is called with the server-side interface of the incoming +connection. It registers the connection and blocks, waiting for a matching tunnel +to proxy the connection through. + +After it has two connections that match, `Connection registry` starts bi-directional +data streaming: -```proto -syntax = "proto3"; - -import "google/protobuf/timestamp.proto"; - -message KasAddress { - string ip = 1; - uint32 port = 2; -} - -message ConnectedAgentInfo { - // Agent id. - int64 id = 1; - // Identifies a particular agentk->kas connection. Randomly generated when agent connects. - int64 connection_id = 2; - string version = 3; - string commit = 4; - // Pod namespace. - string pod_namespace = 5; - // Pod name. - string pod_name = 6; - // When the connection was established. - google.protobuf.Timestamp connected_at = 7; - KasAddress kas_address = 8; - // What else do we need? -} - -message KasInstanceInfo { - string version = 1; - string commit = 2; - KasAddress address = 3; - // What else do we need? -} - -message ConnectedAgentsForProjectRequest { - int64 project_id = 1; -} - -message ConnectedAgentsForProjectResponse { - // There may 0 or more agents with the same id, depending on the number of running Pods. - repeated ConnectedAgentInfo agents = 1; -} - -message ConnectedAgentsByIdRequest { - int64 agent_id = 1; -} - -message ConnectedAgentsByIdResponse { - repeated ConnectedAgentInfo agents = 1; -} - -// API for use by GitLab. -service KasApi { - // Connected agents for a particular configuration project. - rpc ConnectedAgentsForProject (ConnectedAgentsForProjectRequest) returns (ConnectedAgentsForProjectResponse) { - } - // Connected agents for a particular agent id. - rpc ConnectedAgentsById (ConnectedAgentsByIdRequest) returns (ConnectedAgentsByIdResponse) { - } - // Depends on the need, but here is the call from the example above. - rpc GetPods (GetPodsRequest) returns (GetPodsResponse) { - } -} - -message Pod { - string namespace = 1; - string name = 2; -} - -message GetPodsRequest { - int64 agent_id = 1; - int64 connection_id = 2; -} - -message GetPodsResponse { - repeated Pod pods = 1; -} - -// Internal API for use by kas for kas -> kas calls. -service KasInternal { - // Depends on the need, but here is the call from the example above. - rpc GetPods (GetPodsRequest) returns (GetPodsResponse) { - } -} +```mermaid +graph TB + subgraph kas + server-tunnel-module[Server tunnel module] + connection-registry[Connection registry] + server-internal-grpc-server[Internal gRPC server] + server-api-grpc-server[Public API gRPC server] + server-module-a[Server side of module A] + server-module-b[Server side of module B] + end + subgraph agentk + agent-internal-grpc-server[Internal gRPC server] + agent-tunnel-module[Agent tunnel module] + agent-module-a[Agent side of module A] + agent-module-b[Agent side of module B] + end + + server-tunnel-module -- "HandleTunnelConnection()" --> connection-registry + server-internal-grpc-server -- "HandleIncomingConnection()" --> connection-registry + server-api-grpc-server -- "HandleIncomingConnection()" --> connection-registry + server-module-a-. expose API on .-> server-internal-grpc-server + server-module-b-. expose API on .-> server-api-grpc-server + + agent-tunnel-module -- "establish tunnel, receive request" --> server-tunnel-module + agent-tunnel-module -- make request --> agent-internal-grpc-server + agent-internal-grpc-server -- request --> agent-module-a + agent-internal-grpc-server -- request --> agent-module-b ``` + +### API definitions + +- [`agent_tracker/agent_tracker.proto`](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/internal/module/agent_tracker/agent_tracker.proto) +- [`agent_tracker/rpc/rpc.proto`](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/internal/module/agent_tracker/rpc/rpc.proto) +- [`reverse_tunnel/rpc/rpc.proto`](https://gitlab.com/gitlab-org/cluster-integration/gitlab-agent/-/blob/master/internal/module/reverse_tunnel/rpc/rpc.proto) diff --git a/doc/development/agent/user_stories.md b/doc/development/agent/user_stories.md index 2929573ffd3..609be47a3cb 100644 --- a/doc/development/agent/user_stories.md +++ b/doc/development/agent/user_stories.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers --- -# Kubernetes Agent user stories **(PREMIUM ONLY)** +# Kubernetes Agent user stories **(PREMIUM SELF)** The [personas in action](https://about.gitlab.com/handbook/marketing/product-marketing/roles-personas/#user-personas) for the Kubernetes Agent are: diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index d73c3a8d6f6..85098689392 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -10,7 +10,10 @@ This document outlines the style guide for the GitLab [GraphQL API](../api/graph ## How GitLab implements GraphQL + We use the [GraphQL Ruby gem](https://graphql-ruby.org/) written by [Robert Mosolgo](https://github.com/rmosolgo/). + +In addition, we have a subscription to [GraphQL Pro](https://www.graphql.pro). For details see [GraphQL Pro subscription](graphql_guide/graphql_pro.md). All GraphQL queries are directed to a single endpoint ([`app/controllers/graphql_controller.rb#execute`](https://gitlab.com/gitlab-org/gitlab/blob/master/app%2Fcontrollers%2Fgraphql_controller.rb)), @@ -21,6 +24,7 @@ which is exposed as an API endpoint at `/api/graphql`. In March 2019, Nick Thomas hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) on the GitLab [GraphQL API](../api/graphql/index.md) to share his domain specific knowledge with anyone who may work in this part of the codebase in the future. You can find the + [recording on YouTube](https://www.youtube.com/watch?v=-9L_1MWrjkg), and the slides on [Google Slides](https://docs.google.com/presentation/d/1qOTxpkTdHIp1CRjuTvO-aXg0_rUtzE3ETfLUdnBB5uQ/edit) and in [PDF](https://gitlab.com/gitlab-org/create-stage/uploads/8e78ea7f326b2ef649e7d7d569c26d56/GraphQL_Deep_Dive__Create_.pdf). @@ -42,6 +46,36 @@ can be shared. It's also possible to add a `private_token` to the query string, or add a `HTTP_PRIVATE_TOKEN` header. +## Limits + +Several limits apply to the GraphQL API and some of these can be overridden +by developers. + +### Max page size + +By default, [connections](#connection-types) can only return +at most a maximum number of records defined in +[`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb) +per page. + +Developers can [specify a custom max page size](#page-size-limit) when defining +a connection. + +### Max complexity + +Complexity is explained [on our client-facing API page](../api/graphql/index.md#max-query-complexity). + +Fields default to adding `1` to a query's complexity score, but developers can +[specify a custom complexity](#field-complexity) when defining a field. + +To estimate the complexity of a query, you can run the +[`gitlab:graphql:analyze`](rake_tasks.md#analyze-graphql-queries) +Rake task. + +### Request timeout + +Requests time out at 30 seconds. + ## Global IDs The GitLab GraphQL API uses Global IDs (i.e: `"gid://gitlab/MyObject/123"`) @@ -281,6 +315,61 @@ Use the functionality the framework provides unless there is a compelling reason For example, instead of `latest_pipeline`, use `pipelines(last: 1)`. +#### Page size limit + +By default, the API returns at most a maximum number of records defined in +[`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb) +per page within a connection and this will also be the default number of records +returned per page if no limiting arguments (`first:` or `last:`) are provided by a client. + +The `max_page_size` argument can be used to specify a different page size limit +for a connection. + +WARNING: +It's better to change the frontend client, or product requirements, to not need large amounts of +records per page than it is to raise the `max_page_size`, as the default is set to ensure +the GraphQL API remains performant. + +For example: + +```ruby +field :tags, + Types::ContainerRepositoryTagType.connection_type, + null: true, + description: 'Tags of the container repository', + max_page_size: 20 +``` + +### Field complexity + +The GitLab GraphQL API uses a _complexity_ score to limit performing overly complex queries. +Complexity is described in [our client documentation](../api/graphql/index.md#max-query-complexity) on the topic. + +Complexity limits are defined in [`app/graphql/gitlab_schema.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/gitlab_schema.rb). + +By default, fields will add `1` to a query's complexity score. This can be overridden by +[providing a custom `complexity`](https://graphql-ruby.org/queries/complexity_and_depth.html) value for a field. + +Developers should specify higher complexity for fields that cause more _work_ to be performed +by the server in order to return data. Fields that represent data that can be returned +with little-to-no _work_, for example in most cases; `id` or `title`, can be given a complexity of `0`. + +### `calls_gitaly` + +Fields that have the potential to perform a [Gitaly](../administration/gitaly/index.md) call when resolving _must_ be marked as +such by passing `calls_gitaly: true` to `field` when defining it. + +For example: + +```ruby +field :blob, type: Types::Snippets::BlobType, + description: 'Snippet blob', + null: false, + calls_gitaly: true +``` + +This will increment the [`complexity` score](#field-complexity) of the field by `1`. + ### Exposing permissions for a type To expose permissions the current user has on a resource, you can call @@ -1599,7 +1688,7 @@ full stack: - An argument or scalar's [`prepare`](#validating-arguments) applies correctly. - Logic in a resolver or mutation's [`#ready?` method](#correct-use-of-resolverready) applies correctly. - An [argument's `default_value`](https://graphql-ruby.org/fields/arguments.html) applies correctly. -- Objects resolve performantly and there are no N+1 issues. +- Objects resolve successfully, and there are no N+1 issues. When adding a query, you can use the `a working graphql query` shared example to test if the query renders valid results. diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index b2c93f16770..dd43281da6d 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -28,7 +28,7 @@ See the [Documentation Style Guide RESTful API page](documentation/restful_api_s ## Methods and parameters description Every method must be described using the [Grape DSL](https://github.com/ruby-grape/grape#describing-methods) -(see +(see [`environments.rb`](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 diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index c661ff3f617..3608636dd55 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Distribution info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/approval_rules.md b/doc/development/approval_rules.md index 542bce1cb97..368987eb85f 100644 --- a/doc/development/approval_rules.md +++ b/doc/development/approval_rules.md @@ -4,7 +4,7 @@ group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Approval Rules **(STARTER)** +# Approval Rules development guide **(FREE)** This document explains the backend design and flow of all related functionality about [merge request approval rules](../user/project/merge_requests/merge_request_approvals.md). @@ -44,8 +44,8 @@ erDiagram ### `Project` and `MergeRequest` `Project` and `MergeRequest` models are defined in `ee/app/models/ee/project.rb` -and `ee/app/models/ee/merge_request.rb`. They extend the non-EE versions since -approval rules is an EE only feature. Associations and other related stuff to +and `ee/app/models/ee/merge_request.rb`. They extend the non-EE versions, because +approval rules are an EE-only feature. Associations and other related stuff to merge request approvals are defined here. ### `ApprovalState` @@ -265,8 +265,8 @@ graph LR ApprovalWrappedRule --> Approval ``` -This flow gets initiated by the frontend component. The data returned will -then be used to display information on the MR widget. +This flow gets initiated by the frontend component. The data returned is +used to display information on the MR widget. ### Approving a merge request @@ -282,5 +282,5 @@ is executed instead. ## TODO -1. Add information related to other rule types (e.g. `code_owner` and `report_approver`). +1. Add information related to other rule types, such as `code_owner` and `report_approver`. 1. Add information about side effects of approving/unapproving merge request. diff --git a/doc/development/architecture.md b/doc/development/architecture.md index f8ab97de848..69055131ae8 100644 --- a/doc/development/architecture.md +++ b/doc/development/architecture.md @@ -91,6 +91,8 @@ The simplest way to ensure this, is to add support for your feature or service t [the official GitLab Helm chart](https://docs.gitlab.com/charts/) or reach out to [the Distribution team](https://about.gitlab.com/handbook/engineering/development/enablement/distribution/#how-to-work-with-distribution). +Refer to the [process for adding new service components](adding_service_component.md) for more details. + ### Simplified component overview This is a simplified architecture diagram that can be used to @@ -225,52 +227,54 @@ Component statuses are linked to configuration documentation for each component. Table description links: - [Omnibus GitLab](https://docs.gitlab.com/omnibus/) +- [GitLab Environment Toolkit (GET)](https://gitlab.com/gitlab-org/quality/gitlab-environment-toolkit) - [GitLab chart](https://docs.gitlab.com/charts/) - [Minikube Minimal](https://docs.gitlab.com/charts/development/minikube/#deploying-gitlab-with-minimal-settings) - [GitLab.com](https://gitlab.com) - [Source](../install/installation.md) - [GDK](https://gitlab.com/gitlab-org/gitlab-development-kit) -| Component | Description | Omnibus GitLab | GitLab chart | Minikube Minimal | GitLab.com | Source | GDK | CE/EE | -|-------------------------------------------------------|----------------------------------------------------------------------|:--------------:|:------------:|:----------------:|:----------:|:------:|:---:|:-------:| -| [Certificate Management](#certificate-management) | TLS Settings, Let's Encrypt | ✅ | ✅ | ⚙ | ✅ | ⚙ | ⚙ | CE & EE | -| [Consul](#consul) | Database node discovery, failover | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | -| [Database Migrations](#database-migrations) | Database migrations | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | -| [Elasticsearch](#elasticsearch) | Improved search within GitLab | ⤓ | ⤓ | ⤓ | ✅ | ⤓ | ⤓ | EE Only | -| [Gitaly](#gitaly) | Git RPC service for handling all Git calls made by GitLab | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | -| [GitLab Exporter](#gitlab-exporter) | Generates a variety of GitLab metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab Geo Node](#gitlab-geo) | Geographically distributed GitLab nodes | ⚙ | ⚙ | ❌ | ✅ | ❌ | ⚙ | EE Only | -| [GitLab Managed Apps](#gitlab-managed-apps) | Deploy Helm, Ingress, Cert-Manager, Prometheus, GitLab Runner, JupyterHub, or Knative to a cluster | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | CE & EE | -| [GitLab Pages](#gitlab-pages) | Hosts static websites | ⚙ | ❌ | ❌ | ✅ | ⚙ | ⚙ | CE & EE | -| [GitLab Kubernetes Agent](#gitlab-kubernetes-agent) | Integrate Kubernetes clusters in a cloud-native way | ⚙ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | EE Only | -| [GitLab self-monitoring: Alertmanager](#alertmanager) | Deduplicates, groups, and routes alerts from Prometheus | ⚙ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Grafana](#grafana) | Metrics dashboard | ✅ | ⚙ | ⤓ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Jaeger](#jaeger) | View traces generated by the GitLab instance | ❌ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | CE & EE | -| [GitLab self-monitoring: Prometheus](#prometheus) | Time-series database, metrics collection, and query service | ✅ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | -| [GitLab self-monitoring: Sentry](#sentry) | Track errors generated by the GitLab instance | ⤓ | ⤓ | ❌ | ✅ | ⤓ | ⤓ | CE & EE | -| [GitLab Shell](#gitlab-shell) | Handles `git` over SSH sessions | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | -| [GitLab Workhorse](#gitlab-workhorse) | Smart reverse proxy, handles large HTTP requests | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | -| [Inbound email (SMTP)](#inbound-email) | Receive messages to update issues | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | -| [Jaeger integration](#jaeger) | Distributed tracing for deployed apps | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | EE Only | -| [LDAP Authentication](#ldap-authentication) | Authenticate users against centralized LDAP directory | ⤓ | ⤓ | ⤓ | ❌ | ⤓ | ⤓ | CE & EE | -| [Mattermost](#mattermost) | Open-source Slack alternative | ⚙ | ⤓ | ⤓ | ⤓ | ❌ | ❌ | CE & EE | -| [MinIO](#minio) | Object storage service | ⤓ | ✅ | ✅ | ✅ | ❌ | ⚙ | CE & EE | -| [NGINX](#nginx) | Routes requests to appropriate components, terminates SSL | ✅ | ✅ | ⚙ | ✅ | ⤓ | ❌ | CE & EE | -| [Node Exporter](#node-exporter) | Prometheus endpoint with system metrics | ✅ | N/A | N/A | ✅ | ❌ | ❌ | CE & EE | -| [Outbound email (SMTP)](#outbound-email) | Send email messages to users | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | -| [Patroni](#patroni) | Manage PostgreSQL HA cluster leader selection and replication | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | -| [PgBouncer Exporter](#pgbouncer-exporter) | Prometheus endpoint with PgBouncer metrics | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | CE & EE | -| [PgBouncer](#pgbouncer) | Database connection pooling, failover | ⚙ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | -| [PostgreSQL Exporter](#postgresql-exporter) | Prometheus endpoint with PostgreSQL metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | -| [PostgreSQL](#postgresql) | Database | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | -| [Praefect](#praefect) | A transparent proxy between any Git client and Gitaly storage nodes. | ✅ | ⚙ | ❌ | ✅ | ⚙ | ✅ | CE & EE | -| [Redis Exporter](#redis-exporter) | Prometheus endpoint with Redis metrics | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | -| [Redis](#redis) | Caching service | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | -| [Registry](#registry) | Container registry, allows pushing and pulling of images | ⚙ | ✅ | ✅ | ✅ | ⤓ | ⚙ | CE & EE | -| [Runner](#gitlab-runner) | Executes GitLab CI/CD jobs | ⤓ | ✅ | ⚙ | ✅ | ⚙ | ⚙ | CE & EE | -| [Sentry integration](#sentry) | Error tracking for deployed apps | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | CE & EE | -| [Sidekiq](#sidekiq) | Background jobs processor | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | CE & EE | -| [Puma (GitLab Rails)](#puma) | Handles requests for the web interface and API | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| Component | Description | Omnibus GitLab | GitLab Environment Toolkit (GET) | GitLab chart | Minikube Minimal | GitLab.com | Source | GDK | CE/EE | +|-------------------------------------------------------|----------------------------------------------------------------------|:--------------:|:--------------:|:------------:|:----------------:|:----------:|:------:|:---:|:-------:| +| [Certificate Management](#certificate-management) | TLS Settings, Let's Encrypt | ✅ | ✅ | ✅ | ⚙ | ✅ | ⚙ | ⚙ | CE & EE | +| [Consul](#consul) | Database node discovery, failover | ⚙ | ✅ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | +| [Database Migrations](#database-migrations) | Database migrations | ✅ | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| [Elasticsearch](#elasticsearch) | Improved search within GitLab | ⤓ | ⚙ | ⤓ | ⤓ | ✅ | ⤓ | ⤓ | EE Only | +| [Gitaly](#gitaly) | Git RPC service for handling all Git calls made by GitLab | ✅ | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| [GitLab Exporter](#gitlab-exporter) | Generates a variety of GitLab metrics | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | +| [GitLab Geo Node](#gitlab-geo) | Geographically distributed GitLab nodes | ⚙ | ⚙ | ❌ | ❌ | ✅ | ❌ | ⚙ | EE Only | +| [GitLab Kubernetes Agent](#gitlab-kubernetes-agent) | Integrate Kubernetes clusters in a cloud-native way | ⚙ | ⚙ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | EE Only | +| [GitLab Managed Apps](#gitlab-managed-apps) | Deploy Helm, Ingress, Cert-Manager, Prometheus, GitLab Runner, JupyterHub, or Knative to a cluster | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | CE & EE | +| [GitLab Pages](#gitlab-pages) | Hosts static websites | ⚙ | ⚙ | ❌ | ❌ | ✅ | ⚙ | ⚙ | CE & EE | +| [GitLab Kubernetes Agent](#gitlab-kubernetes-agent) | Integrate Kubernetes clusters in a cloud-native way | ⚙ | ⚙ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | EE Only | +| [GitLab self-monitoring: Alertmanager](#alertmanager) | Deduplicates, groups, and routes alerts from Prometheus | ⚙ | ⚙ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | +| [GitLab self-monitoring: Grafana](#grafana) | Metrics dashboard | ✅ | ✅ | ⚙ | ⤓ | ✅ | ❌ | ❌ | CE & EE | +| [GitLab self-monitoring: Jaeger](#jaeger) | View traces generated by the GitLab instance | ❌ | ⚙ | ⚙ | ❌ | ❌ | ⤓ | ⚙ | CE & EE | +| [GitLab self-monitoring: Prometheus](#prometheus) | Time-series database, metrics collection, and query service | ✅ | ✅ | ✅ | ⚙ | ✅ | ❌ | ❌ | CE & EE | +| [GitLab self-monitoring: Sentry](#sentry) | Track errors generated by the GitLab instance | ⤓ | ⤓ | ⤓ | ❌ | ✅ | ⤓ | ⤓ | CE & EE | +| [GitLab Shell](#gitlab-shell) | Handles `git` over SSH sessions | ✅ | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| [GitLab Workhorse](#gitlab-workhorse) | Smart reverse proxy, handles large HTTP requests | ✅ | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| [Inbound email (SMTP)](#inbound-email) | Receive messages to update issues | ⤓ | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | +| [Jaeger integration](#jaeger) | Distributed tracing for deployed apps | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | EE Only | +| [LDAP Authentication](#ldap-authentication) | Authenticate users against centralized LDAP directory | ⤓ | ⤓ | ⤓ | ⤓ | ❌ | ⤓ | ⤓ | CE & EE | +| [Mattermost](#mattermost) | Open-source Slack alternative | ⚙ | ⚙ | ⤓ | ⤓ | ⤓ | ❌ | ❌ | CE & EE | +| [MinIO](#minio) | Object storage service | ⤓ | ⤓ | ✅ | ✅ | ✅ | ❌ | ⚙ | CE & EE | +| [NGINX](#nginx) | Routes requests to appropriate components, terminates SSL | ✅ | ✅ | ✅ | ⚙ | ✅ | ⤓ | ❌ | CE & EE | +| [Node Exporter](#node-exporter) | Prometheus endpoint with system metrics | ✅ | ✅ | N/A | N/A | ✅ | ❌ | ❌ | CE & EE | +| [Outbound email (SMTP)](#outbound-email) | Send email messages to users | ⤓ | ⤓ | ⚙ | ⤓ | ✅ | ⤓ | ⤓ | CE & EE | +| [Patroni](#patroni) | Manage PostgreSQL HA cluster leader selection and replication | ⚙ | ✅ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | +| [PgBouncer Exporter](#pgbouncer-exporter) | Prometheus endpoint with PgBouncer metrics | ⚙ | ✅ | ❌ | ❌ | ✅ | ❌ | ❌ | CE & EE | +| [PgBouncer](#pgbouncer) | Database connection pooling, failover | ⚙ | ✅ | ❌ | ❌ | ✅ | ❌ | ❌ | EE Only | +| [PostgreSQL Exporter](#postgresql-exporter) | Prometheus endpoint with PostgreSQL metrics | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | +| [PostgreSQL](#postgresql) | Database | ✅ | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | +| [Praefect](#praefect) | A transparent proxy between any Git client and Gitaly storage nodes. | ✅ | ✅ | ⚙ | ❌ | ✅ | ⚙ | ✅ | CE & EE | +| [Puma (GitLab Rails)](#puma) | Handles requests for the web interface and API | ✅ | ✅ | ✅ | ✅ | ✅ | ⚙ | ✅ | CE & EE | +| [Redis Exporter](#redis-exporter) | Prometheus endpoint with Redis metrics | ✅ | ✅ | ✅ | ✅ | ✅ | ❌ | ❌ | CE & EE | +| [Redis](#redis) | Caching service | ✅ | ✅ | ✅ | ✅ | ✅ | ⤓ | ✅ | CE & EE | +| [Registry](#registry) | Container registry, allows pushing and pulling of images | ⚙ | ⚙ | ✅ | ✅ | ✅ | ⤓ | ⚙ | CE & EE | +| [Runner](#gitlab-runner) | Executes GitLab CI/CD jobs | ⤓ | ⤓ | ✅ | ⚙ | ✅ | ⚙ | ⚙ | CE & EE | +| [Sentry integration](#sentry) | Error tracking for deployed apps | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | ⤓ | CE & EE | +| [Sidekiq](#sidekiq) | Background jobs processor | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | CE & EE | ### Component details @@ -934,7 +938,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` Puma. All these components should run as different system users to GitLab (for example, `postgres`, `redis`, and `www-data`, instead of `git`). diff --git a/doc/development/auto_devops.md b/doc/development/auto_devops.md index c457573b87a..eaf1d712f17 100644 --- a/doc/development/auto_devops.md +++ b/doc/development/auto_devops.md @@ -4,7 +4,7 @@ group: Configure info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Auto DevOps development guide +# Auto DevOps development guide **(FREE)** This document provides a development guide for contributors to [Auto DevOps](../topics/autodevops/index.md). diff --git a/doc/development/changelog.md b/doc/development/changelog.md index 8fad32ed163..f2c8aa4db62 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -45,7 +45,7 @@ the `author` field. GitLab team members **should not**. **must** have a changelog entry, without `merge_request` value and with `type` set to `security`. - Any user-facing change **must** have a changelog entry. This includes both visual changes (regardless of how minor), and changes to the rendered DOM which impact how a screen reader may announce the content. -- Any client-facing change to our REST and GraphQL APIs **must** have a changelog entry. +- Any client-facing change to our REST and GraphQL APIs **must** have a changelog entry. This includes modifying complexity of GraphQL fields. - Performance improvements **should** have a changelog entry. - Changes that need to be documented in the Product Intelligence [Event Dictionary](https://about.gitlab.com/handbook/product/product-intelligence-guide/#event-dictionary) also require a changelog entry. @@ -168,6 +168,7 @@ type: | [`--dry-run`](#--dry-run-or--n) | `-n` | Don't actually write anything, just print | | [`--git-username`](#--git-username-or--u) | `-u` | Use Git user.name configuration as the author | | [`--type`](#--type-or--t) | `-t` | The category of the change, valid options are: `added`, `fixed`, `changed`, `deprecated`, `removed`, `security`, `performance`, `other` | +| [`--ee`](#how-to-generate-a-changelog-entry) | | Create an EE changelog | `--help` | `-h` | Print help message | #### `--amend` @@ -274,6 +275,20 @@ author: type: added ``` +#### `--ee` + +Use the **`--ee`** argument to create an EE changelog: + +```plaintext +$ bin/changelog 'Hey DZ, I added a feature to GitLab!' -ee +create ee/changelogs/unreleased/feature-hey-dz.yml +--- +title: Hey DZ, I added a feature to GitLab! +merge_request: +author: +type: added +``` + ### History and Reasoning Our `CHANGELOG` file was previously updated manually by each contributor that diff --git a/doc/development/chaos_endpoints.md b/doc/development/chaos_endpoints.md index 9104c01c980..85c93f521ac 100644 --- a/doc/development/chaos_endpoints.md +++ b/doc/development/chaos_endpoints.md @@ -6,8 +6,12 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Generating chaos in a test GitLab instance + + As [Werner Vogels](https://twitter.com/Werner), the CTO at Amazon Web Services, famously put it, **Everything fails, all the time**. + + As a developer, it's as important to consider the failure modes in which your software may operate as much as normal operation. Doing so can mean the difference between a minor hiccup leading to a scattering of `500` errors experienced by a tiny fraction of users, and a full site outage that affects all users for an extended period. To paraphrase [Tolstoy](https://en.wikipedia.org/wiki/Anna_Karenina_principle), _all happy servers are alike, but all failing servers are failing in their own way_. Luckily, there are ways we can attempt to simulate these failure modes, and the chaos endpoints are tools for assisting in this process. @@ -160,3 +164,58 @@ GET /-/chaos/kill?async=true curl "http://localhost:3000/-/chaos/kill" --header 'X-Chaos-Secret: secret' curl "http://localhost:3000/-/chaos/kill?token=secret" ``` + +## Run garbage collector + +This endpoint triggers a GC run on the worker handling the request and returns its worker ID +plus GC stats as JSON. This is mostly useful when running Puma in standalone mode, since +otherwise the worker handling the request will not be known upfront. + +Endpoint: + +```plaintext +POST /-/chaos/gc +``` + +Example request: + +```shell +curl --request POST "http://localhost:3000/-/chaos/gc" --header 'X-Chaos-Secret: secret' +curl --request POST "http://localhost:3000/-/chaos/gc?token=secret" +``` + +Example response: + +```json +{ + "worker_id": "puma_1", + "gc_stat": { + "count": 94, + "heap_allocated_pages": 9077, + "heap_sorted_length": 9077, + "heap_allocatable_pages": 0, + "heap_available_slots": 3699720, + "heap_live_slots": 2827510, + "heap_free_slots": 872210, + "heap_final_slots": 0, + "heap_marked_slots": 2827509, + "heap_eden_pages": 9077, + "heap_tomb_pages": 0, + "total_allocated_pages": 9077, + "total_freed_pages": 0, + "total_allocated_objects": 14229357, + "total_freed_objects": 11401847, + "malloc_increase_bytes": 8192, + "malloc_increase_bytes_limit": 30949538, + "minor_gc_count": 71, + "major_gc_count": 23, + "compact_count": 0, + "remembered_wb_unprotected_objects": 41685, + "remembered_wb_unprotected_objects_limit": 83370, + "old_objects": 2617806, + "old_objects_limit": 5235612, + "oldmalloc_increase_bytes": 8192, + "oldmalloc_increase_bytes_limit": 122713697 + } +} +``` diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index a2a0005f7cb..0341abf5eeb 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/doc/development/chatops_on_gitlabcom.md @@ -35,14 +35,12 @@ To request access to ChatOps on GitLab.com: in the `#chat-ops-test` Slack channel, replacing `` with your username: `/chatops run member add gitlab-com/chatops --ops` - - - > Hi `__BUDDY_HANDLE__` and `__MANAGER_HANDLE__`, could you please add me to - > the ChatOps project in Ops by running this command: - > `/chatops run member add gitlab-com/chatops --ops` in the - > `#chat-ops-test` Slack channel? Thanks in advance. - - + ```plaintext + Hi <__BUDDY_HANDLE__> and <__MANAGER_HANDLE__>, could you please add me to + the ChatOps project in Ops by running this command: + `/chatops run member add gitlab-com/chatops --ops` in the + `#chat-ops-test` Slack channel? Thanks in advance. + ``` 1. Ensure you've set up two-factor authentication. 1. After you're added to the ChatOps project, run this command to check your user @@ -61,6 +59,6 @@ To request access to ChatOps on GitLab.com: ## See also -- [ChatOps Usage](../ci/chatops/README.md) +- [ChatOps Usage](../ci/chatops/index.md) - [Understanding EXPLAIN plans](understanding_explain_plans.md) - [Feature Groups](feature_flags/development.md#feature-groups) diff --git a/doc/development/cicd/img/ci_minutes.png b/doc/development/cicd/img/ci_minutes.png new file mode 100644 index 00000000000..d47406fb445 Binary files /dev/null and b/doc/development/cicd/img/ci_minutes.png differ diff --git a/doc/development/cicd/index.md b/doc/development/cicd/index.md index eede1d691a9..eb2224d710a 100644 --- a/doc/development/cicd/index.md +++ b/doc/development/cicd/index.md @@ -143,3 +143,42 @@ Finally if the runner can only pick jobs that are tagged, all untagged jobs are At this point we loop through remaining `pending` jobs and we try to assign the first job that the runner "can pick" based on additional policies. For example, runners marked as `protected` can only pick jobs that run against protected branches (such as production deployments). As we increase the number of runners in the pool we also increase the chances of conflicts which would arise if assigning the same job to different runners. To prevent that we gracefully rescue conflict errors and assign the next job in the list. + +## The definition of "Job" in GitLab CI/CD + +"Job" in GitLab CI context refers a task to drive Continuous Integration, Delivery and Deployment. +Typically, a pipeline contains multiple stages, and a stage contains multiple jobs. + +In Active Record modeling, Job is defined as `CommitStatus` class. +On top of that, we have the following types of jobs: + +- `Ci::Build` ... The job to be executed by runners. +- `Ci::Bridge` ... The job to trigger a downstream pipeline. +- `GenericCommitStatus` ... The job to be executed in an external CI/CD system e.g. Jenkins. + +Please note that, when you use the "Job" terminology in codebase, readers would +assume that the class/object is any type of above. +If you specifically refer `Ci::Build` class, you should not name the object/class +as "job" as this could cause some confusions. In documentation, +we should use "Job" in general, instead of "Build". + +We have a few inconsistencies in our codebase that should be refactored. +For example, `CommitStatus` should be `Ci::Job` and `Ci::JobArtifact` should be `Ci::BuildArtifact`. +See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/16111) for the full refactoring plan. + +## CI Minutes + +This diagram shows how the [CI minutes](../../subscriptions/gitlab_com/index.md#ci-pipeline-minutes) +feature and its components work. + +![CI Minutes architecture](img/ci_minutes.png) + + +Watch a walkthrough of this feature in details in the video below. + +
+ See the video: CI Minutes - architectural overview. +
+
+ +
diff --git a/doc/development/code_intelligence/index.md b/doc/development/code_intelligence/index.md index c5673f6eee2..ac962e3ae3e 100644 --- a/doc/development/code_intelligence/index.md +++ b/doc/development/code_intelligence/index.md @@ -4,7 +4,7 @@ group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Code Intelligence +# Code Intelligence **(FREE)** > [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/1576) in GitLab 13.1. @@ -45,7 +45,7 @@ sequenceDiagram GitLab Rails to authorize the upload. 1. GitLab Rails validates whether the artifact can be uploaded and sends - `ProcessLsif: true` header if the lsif artifact can be processed. + `ProcessLsif: true` header if the LSIF artifact can be processed. 1. Workhorse reads the LSIF document line by line and generates code intelligence data for each file in the project. The output is a zipped directory of JSON diff --git a/doc/development/code_review.md b/doc/development/code_review.md index fe395dc2304..dada6adcce7 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -24,7 +24,7 @@ uncovered edge cases. The default approach is to choose a reviewer from your group or team for the first review. This is only a recommendation and the reviewer may be from a different team. However, it is recommended to pick someone who is a [domain expert](#domain-experts). -If your merge request touches more than one domain (for example, Dynamic Analysis and GraphQL), ask for reviews from an expert from each domain. +If your merge request touches more than one domain (for example, Dynamic Analysis and GraphQL), ask for reviews from an expert from each domain. You can read more about the importance of involving reviewer(s) in the section on the responsibility of the author below. @@ -107,11 +107,11 @@ with [domain expertise](#domain-experts). be **approved by a [frontend foundations member](https://about.gitlab.com/direction/create/ecosystem/frontend-ux-foundations/)**. - If the license used by the new library hasn't been approved for use in GitLab, the license must be **approved by a [legal department member](https://about.gitlab.com/handbook/legal/)**. - More information about license compatiblity can be found in our + More information about license compatibility can be found in our [GitLab Licensing and Compatibility documentation](licensing.md). 1. If your merge request includes adding a new UI/UX paradigm (*1*), it must be **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 +1. If your merge request includes a new dependency or a file system change, it must be **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. 1. If your merge request includes documentation changes, it must be **approved by a [Technical writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments)**, based on @@ -121,6 +121,8 @@ with [domain expertise](#domain-experts). 1. If your merge request only includes end-to-end changes (*3*) **or** if the MR author is a [Software Engineer in Test](https://about.gitlab.com/handbook/engineering/quality/#individual-contributors), it must be **approved by a [Quality maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_qa)** 1. If your merge request includes a new or updated [application limit](https://about.gitlab.com/handbook/product/product-processes/#introducing-application-limits), it must be **approved by a [product manager](https://about.gitlab.com/company/team/)**. 1. If your merge request includes Product Intelligence (telemetry or analytics) changes, it should be reviewed and approved by a [Product Intelligence engineer](https://gitlab.com/gitlab-org/growth/product_intelligence/engineers). +1. If your merge request includes an addition of, or changes to a [Feature spec](testing_guide/testing_levels.md#frontend-feature-tests), it must be **approved by a [Quality maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_qa) or [Quality reviewer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_reviewers_qa)**. +1. If your merge request introduces a new service to GitLab (Puma, Sidekiq, Gitaly are examples), it must be **approved by a [product manager](https://about.gitlab.com/company/team/)**. See the [process for adding a service component to GitLab](adding_service_component.md) for details. - (*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 @@ -177,8 +179,11 @@ warrant a comment could be: Avoid: -- Adding comments (referenced above, or TODO items) directly to the source code unless the reviewer requires you to do so. If the comments are added due to an actionable task, -a link to an issue must be included. +- Adding TODO comments (referenced above) directly to the source code unless the reviewer requires + you to do so. If TODO comments are added due to an actionable task, + [include a link to the relevant issue](code_comments.md). +- Adding comments which only explain what the code is doing. If non-TODO comments are added, they should + [_explain why, not what_](https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/). - Assigning merge requests with failed tests to maintainers. If the tests are failing and you have to assign, ensure you leave a comment with an explanation. - Excessively mentioning maintainers through email or Slack (if the maintainer is reachable through Slack). If you can't assign a merge request, `@` mentioning a maintainer in a comment is acceptable and in all other cases assigning the merge request is sufficient. @@ -276,10 +281,9 @@ first time. of your shiny new branch, read through the entire diff. Does it make sense? Did you include something unrelated to the overall purpose of the changes? Did you forget to remove any debugging code? - -- Be grateful for the reviewer's suggestions. ("Good call. I'll make that - change.") - +- Consider providing instructions on how to test the merge request. This can be + helpful for reviewers not familiar with the product feature or area of the codebase. +- Be grateful for the reviewer's suggestions. (`Good call. I'll make that change.`) - Don't take it personally. The review is of the code, not of you. - Explain why the code exists. ("It's like that because of these reasons. Would it be more clear if I rename this class/file/method/variable?") @@ -340,7 +344,7 @@ experience, refactors the existing code). Then: convey your intent. - For non-mandatory suggestions, decorate with (non-blocking) so the author knows they can optionally resolve within the merge request or follow-up at a later stage. - - There's a [Chrome/Firefox addon](https://gitlab.com/conventionalcomments/conventional-comments-button) which you can use to apply [Conventional Comment](https://conventionalcomments.org/) prefixes. + - There's a [Chrome/Firefox add-on](https://gitlab.com/conventionalcomments/conventional-comments-button) which you can use to apply [Conventional Comment](https://conventionalcomments.org/) prefixes. - After a round of line notes, it can be helpful to post a summary note such as "Looks good to me", or "Just a couple things to address." - Assign the merge request to the author if changes are required following your @@ -378,9 +382,12 @@ When ready to merge: - Consider using the [Squash and merge](../user/project/merge_requests/squash_and_merge.md#squash-and-merge) feature when the merge request has a lot of commits. - When merging code a maintainer should only use the squash feature if the - author has already set this option or if the merge request clearly contains a - messy commit history that is intended to be squashed. + When merging code, a maintainer should only use the squash feature if the + author has already set this option, or if the merge request clearly contains a + messy commit history, it will be more efficient to squash commits instead of + circling back with the author about that. Otherwise, if the MR only has a few commits, we'll + be respecting the author's setting by not squashing them. + - **Start a new merge request pipeline with the `Run Pipeline` button in the merge request's "Pipelines" tab, and enable "Merge When Pipeline Succeeds" (MWPS).** Note that: - If the **latest [Pipeline for Merged Results](../ci/merge_request_pipelines/pipelines_for_merged_results/#pipelines-for-merged-results)** finished less than 2 hours ago, you @@ -565,7 +572,7 @@ A good example of collaboration on an MR touching multiple parts of the codebase ### Credits -Largely based on the [thoughtbot code review guide](https://github.com/thoughtbot/guides/tree/master/code-review). +Largely based on the [`thoughtbot` code review guide](https://github.com/thoughtbot/guides/tree/master/code-review). --- diff --git a/doc/development/contributing/index.md b/doc/development/contributing/index.md index 329303558b0..7a1c189e087 100644 --- a/doc/development/contributing/index.md +++ b/doc/development/contributing/index.md @@ -181,11 +181,21 @@ reasons for including it. `@mention` a maintainer in merge requests that contain: - More than 500 changes. -- Any major breaking changes. +- Any major [breaking changes](#breaking-changes). - External libraries. If you are not sure who to mention, the reviewer will do this for you early in the merge request process. +#### Breaking changes + +A "breaking change" is any change that requires users to make a corresponding change to their code, settings, or workflow. "Users" might be humans, API clients, or even code classes that "use" another class. Examples of breaking changes include: + +- Removing a user-facing feature without a replacement/workaround. +- Changing the definition of an existing API (by re-naming query parameters, changing routes, etc.). +- Removing a public method from a code class. + +A breaking change can be considered "major" if it affects many users, or represents a significant change in behavior. + #### Issues workflow This [documentation](issue_workflow.md) outlines the current issue workflow: diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md index 7859af4e88b..166f7b350bf 100644 --- a/doc/development/contributing/merge_request_workflow.md +++ b/doc/development/contributing/merge_request_workflow.md @@ -65,9 +65,9 @@ request is as follows: template already provided in the "Description" field. 1. If you are contributing documentation, choose `Documentation` from the "Choose a template" menu and fill in the description according to the template. - 1. Mention the issue(s) your merge request solves, using the `Solves #XXX` or - `Closes #XXX` syntax to [auto-close](../../user/project/issues/managing_issues.md#closing-issues-automatically) - the issue(s) once the merge request is merged. + 1. Use the syntax `Solves #XXX`, `Closes #XXX`, or `Refs #XXX` to mention the issue(s) your merge + request addresses. Referenced issues do not [close automatically](../../user/project/issues/managing_issues.md#closing-issues-automatically). + You must close them manually once the merge request is merged. 1. If you're allowed to, set a relevant milestone and [labels](issue_workflow.md). 1. UI changes should use available components from the GitLab Design System, [Pajamas](https://design.gitlab.com/). The MR must include *Before* and @@ -150,7 +150,7 @@ Commit messages should follow the guidelines below, for reasons explained by Chr #### Why these standards matter 1. Consistent commit messages that follow these guidelines make the history more readable. -1. Concise standard commit messages helps to identify breaking changes for a deployment or ~"master:broken" quicker when +1. Concise standard commit messages helps to identify [breaking changes](index.md#breaking-changes) for a deployment or ~"master:broken" quicker when reviewing commits between two points in time. #### Commit message template @@ -212,7 +212,7 @@ the contribution acceptance criteria below: 1. Changes do not degrade performance: - Avoid repeated polling of endpoints that require a significant amount of overhead. - Check for N+1 queries via the SQL log or [`QueryRecorder`](../merge_request_performance_guidelines.md). - - Avoid repeated access of the filesystem. + - Avoid repeated access of the file system. - Use [polling with ETag caching](../polling.md) if needed to support real-time features. 1. If the merge request adds any new libraries (gems, JavaScript libraries, etc.), they should conform to our [Licensing guidelines](../licensing.md). See those diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index c316d50c88c..2a2cfebe964 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -15,52 +15,83 @@ settings automatically by default. If your editor/IDE does not automatically sup we suggest investigating to see if a plugin exists. For instance here is the [plugin for vim](https://github.com/editorconfig/editorconfig-vim). -## Pre-push static analysis +## Pre-push static analysis with Lefthook -We strongly recommend installing [Lefthook](https://github.com/Arkweid/lefthook) to automatically check -for static analysis offenses before pushing your changes. +[Lefthook](https://github.com/Arkweid/lefthook) is a Git hooks manager that allows +custom logic to be executed prior to Git committing or pushing. GitLab comes with +Lefthook configuration (`lefthook.yml`), but it must be installed. -To install `lefthook`, run the following in your GitLab source directory: +We have a `lefthook.yml` checked in but it is ignored until Lefthook is installed. -```shell -# 1. Make sure to uninstall Overcommit first -overcommit --uninstall +### Uninstall Overcommit + +We were using Overcommit prior to Lefthook, so you may want to uninstall it first with `overcommit --uninstall`. + +### Install Lefthook + +1. Install the `lefthook` Ruby gem: + + ```shell + bundle install + ``` + +1. Install Lefthook managed Git hooks: + + ```shell + bundle exec lefthook install + ``` + +1. Test Lefthook is working by running the Lefthook `prepare-commit-msg` Git hook: + + ```shell + bundle exec lefthook run prepare-commit-msg + ``` + +This should return a fully qualified path command with no other output. + +### Lefthook configuration -# If using rbenv, at this point you may need to do: rbenv rehash +The current Lefthook configuration can be found in [`lefthook.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lefthook.yml). -# 2. Install lefthook... +Before you push your changes, Lefthook automatically runs the following checks: -## With Homebrew (macOS) -brew install Arkweid/lefthook/lefthook +- Danger: Runs a subset of checks that `danger-review` runs on your merge requests. +- ES lint: Run `yarn eslint` checks (with the [`.eslintrc.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.eslintrc.yml) configuration) on the modified `*.{js,vue}` files. Tags: `frontend`, `style`. +- HAML lint: Run `bundle exec haml-lint` checks (with the [`.haml-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.haml-lint.yml) configuration) on the modified `*.html.haml` files. Tags: `view`, `haml`, `style`. +- Markdown lint: Run `yarn markdownlint` checks on the modified `*.md` files. Tags: `documentation`, `style`. +- SCSS lint: Run `bundle exec scss-lint` checks (with the [`.scss-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.scss-lint.yml) configuration) on the modified `*.scss{,.css}` files. Tags: `stylesheet`, `css`, `style`. +- RuboCop: Run `bundle exec rubocop` checks (with the [`.rubocop.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.rubocop.yml) configuration) on the modified `*.rb` files. Tags: `backend`, `style`. +- Vale: Run `vale` checks (with the [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.vale.ini) configuration) on the modified `*.md` files. Tags: `documentation`, `style`. -## Or with Go -go get github.com/Arkweid/lefthook +In addition to the default configuration, you can define a [local configuration](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#local-config). -## Or with Rubygems -gem install lefthook +### Disable Lefthook temporarily -### You may need to run the following if you're using rbenv -rbenv rehash +To disable Lefthook temporarily, you can set the `LEFTHOOK` environment variable to `0`. For instance: -# 3. Install the Git hooks -lefthook install -f +```shell +LEFTHOOK=0 git push ... ``` -Before you push your changes, Lefthook then automatically run Danger checks, and other checks -for changed files. This saves you time as you don't have to wait for the same errors to be detected -by CI/CD. +### Run Lefthook hooks manually + +To run the `pre-push` Git hook, run: + +```shell +bundle exec lefthook run pre-push +``` -Lefthook relies on a pre-push hook to prevent commits that violate its ruleset. -To override this behavior, pass the environment variable `LEFTHOOK=0`. That is, -`LEFTHOOK=0 git push`. +For more information, check out [Lefthook documentation](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#run-githook-group-directly). -You can also: +### Skip Lefthook checks per tag -- Define [local configuration](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#local-config). -- Skip [checks per tag on the fly](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#skip-some-tags-on-the-fly). - For example, `LEFTHOOK_EXCLUDE=frontend git push origin`. -- Run [hooks manually](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#run-githook-group-directly). - For example, `lefthook run pre-push`. +To skip some checks based on tags when pushing, you can set the `LEFTHOOK_EXCLUDE` environment variable. For instance: + +```shell +LEFTHOOK_EXCLUDE=frontend,documentation git push ... +``` + +For more information, check out [Lefthook documentation](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#skip-some-tags-on-the-fly). ## Ruby, Rails, RSpec @@ -111,7 +142,7 @@ This ensures that our list isn't mistakenly removed by another auto generation o the `.rubocop_todo.yml`. This also allows us greater visibility into the exceptions which are currently being resolved. -One way to generate the initial list is to run the todo auto generation, +One way to generate the initial list is to run the `todo` auto generation, with `exclude limit` set to a high number. ```shell @@ -149,8 +180,12 @@ See the dedicated [Shell scripting standards and style guidelines](../shell_scri ## Markdown + + We're following [Ciro Santilli's Markdown Style Guide](https://cirosantilli.com/markdown-style-guide/). + + ## Documentation See the dedicated [Documentation Style Guide](../documentation/styleguide/index.md). diff --git a/doc/development/cycle_analytics.md b/doc/development/cycle_analytics.md deleted file mode 100644 index 1619f3dcb10..00000000000 --- a/doc/development/cycle_analytics.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -redirect_to: 'value_stream_analytics.md' ---- - -This document was moved to [another location](value_stream_analytics.md) - - - diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md index 59b31437161..413c0a31eec 100644 --- a/doc/development/dangerbot.md +++ b/doc/development/dangerbot.md @@ -105,9 +105,9 @@ minimize the number of lines of code in `danger/`. A non-trivial `Dangerfile` should mostly call plugin code with arguments derived from the methods provided by Danger. The plugin code itself should have unit tests. -At present, we do this by putting the code in a module in `lib/gitlab/danger/...`, +At present, we do this by putting the code in a module in `tooling/danger/...`, and including it in the matching `danger/plugins/...` file. Specs can then be -added in `spec/lib/gitlab/danger/...`. +added in `spec/tooling/danger/...`. To determine if your `Dangerfile` works, push the branch that contains it to GitLab. This can be quite frustrating, as it significantly increases the cycle diff --git a/doc/development/database/database_reviewer_guidelines.md b/doc/development/database/database_reviewer_guidelines.md index 26083183d6d..a242a3d5fd0 100644 --- a/doc/development/database/database_reviewer_guidelines.md +++ b/doc/development/database/database_reviewer_guidelines.md @@ -50,17 +50,17 @@ European/US and APAC friendly hours. You can join the office hours call and brin that require a more in-depth discussion between the database reviewers and maintainers: - [Database Office Hours Agenda](https://docs.google.com/document/d/1wgfmVL30F8SdMg-9yY6Y8djPSxWNvKmhR5XmsvYX1EI/edit). -- [Youtube playlist with past recordings](https://www.youtube.com/playlist?list=PL05JrBw4t0Kp-kqXeiF7fF7cFYaKtdqXM). +- [YouTube playlist with past recordings](https://www.youtube.com/playlist?list=PL05JrBw4t0Kp-kqXeiF7fF7cFYaKtdqXM). You should also join the [#database-labs](../understanding_explain_plans.md#database-lab) -Slack channel and get familiar with how to use Joe, the slackbot that provides developers +Slack channel and get familiar with how to use Joe, the Slackbot that provides developers with their own clone of the production database. Understanding and efficiently using `EXPLAIN` plans is at the core of the database review process. The following guides provide a quick introduction and links to follow on more advanced topics: - Guide on [understanding EXPLAIN plans](../understanding_explain_plans.md). -- [Explaining the unexplainable series in depesz](https://www.depesz.com/tag/unexplainable/). +- [Explaining the unexplainable series in `depesz`](https://www.depesz.com/tag/unexplainable/). Finally, you can find various guides in the [Database guides](index.md) page that cover more specific topics and use cases. The most frequently required during database reviewing are the following: diff --git a/doc/development/database/strings_and_the_text_data_type.md b/doc/development/database/strings_and_the_text_data_type.md index 33a0fd2ebb7..f338520c6ca 100644 --- a/doc/development/database/strings_and_the_text_data_type.md +++ b/doc/development/database/strings_and_the_text_data_type.md @@ -34,6 +34,12 @@ but only for updating the declaration of the columns. We can then validate it at `VALIDATE CONSTRAINT`, which requires only a `SHARE UPDATE EXCLUSIVE LOCK` (only conflicts with other validations and index creation while it allows reads and writes). +### Exceptions + +Text columns used by `attr_encrypted` are not required to have a limit, because the length of the +text after encryption may be longer than the text itself. Instead, you can use an Active Record +length validation on the attribute. + ## Create a new table with text columns When adding a new table, the limits for all text columns should be added in the same migration as diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index 358b9bb42b0..207d5a733ce 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -60,7 +60,7 @@ was the first table to be partitioned in the application database (scheduled for deployment with the GitLab 13.5 release). This table tracks audit entries of security events that happen in the application. In almost all cases, users want to see audit activity that -occurs in a certain timeframe. As a result, date-range partitioning +occurs in a certain time frame. As a result, date-range partitioning was a natural fit for how the data would be accessed. To look at this in more detail, imagine a simplified `audit_events` schema: diff --git a/doc/development/database_debugging.md b/doc/development/database_debugging.md index f4558189000..dc64b59018b 100644 --- a/doc/development/database_debugging.md +++ b/doc/development/database_debugging.md @@ -69,16 +69,14 @@ bundle exec rails db -e development Use these instructions for exploring the GitLab database while developing with the GDK: 1. Install or open [Visual Studio Code](https://code.visualstudio.com/download). -1. Install the [PostgreSQL VSCode Extension](https://marketplace.visualstudio.com/items?itemName=ckolkman.vscode-postgres) by Chris Kolkman. +1. Install the [PostgreSQL VSCode Extension](https://marketplace.visualstudio.com/items?itemName=ckolkman.vscode-postgres). 1. In Visual Studio Code click on the PostgreSQL Explorer button in the left toolbar. 1. In the top bar of the new window, click on the `+` to **Add Database Connection**, and follow the prompts to fill in the details: 1. **Hostname**: the path to the PostgreSQL folder in your GDK directory (for example `/dev/gitlab-development-kit/postgresql`). 1. **PostgreSQL user to authenticate as**: usually your local username, unless otherwise specified during PostgreSQL installation. 1. **Password of the PostgreSQL user**: the password you set when installing PostgreSQL. 1. **Port number to connect to**: `5432` (default). - 1. - **Use an ssl connection?** - This depends on your installation. Options are: + 1. **Use an SSL connection?** This depends on your installation. Options are: - **Use Secure Connection** - **Standard Connection** (default) 1. **(Optional) The database to connect to**: `gitlabhq_development`. diff --git a/doc/development/database_review.md b/doc/development/database_review.md index da2c93cc1fd..a19f46b2198 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -144,10 +144,13 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac ##### Query Plans - The query plan for each raw SQL query included in the merge request along with the link to the query plan following each raw SQL snippet. -- Provide the link to the plan at: [explain.depesz.com](https://explain.depesz.com). Paste both the plan and the query used in the form. +- Provide a public link to the plan from either: + - [postgres.ai](https://postgres.ai/): Follow the link in `#database-lab` and generate a shareable, public link + by clicking the **Share** button in the upper right corner. + - [explain.depesz.com](https://explain.depesz.com): Paste both the plan and the query used in the form. - When providing query plans, make sure it hits enough data: - You can use a GitLab production replica to test your queries on a large scale, - through the `#database-lab` Slack channel or through [chatops](understanding_explain_plans.md#chatops). + through the `#database-lab` Slack channel or through [ChatOps](understanding_explain_plans.md#chatops). - 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. @@ -220,13 +223,13 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac - Check for any obviously complex queries and queries the author specifically points out for review (if any) - If not present yet, ask the author to provide SQL queries and query plans - (for example, by using [chatops](understanding_explain_plans.md#chatops) or direct + (for example, by using [ChatOps](understanding_explain_plans.md#chatops) or direct database access) - For given queries, review parameters regarding data distribution - [Check query plans](understanding_explain_plans.md) and suggest improvements to queries (changing the query, schema or adding indexes and similar) - General guideline is for queries to come in below [100ms execution time](query_performance.md#timing-guidelines-for-queries) - - Avoid N+1 problems and minimalize the [query count](merge_request_performance_guidelines.md#query-counts). + - Avoid N+1 problems and minimize the [query count](merge_request_performance_guidelines.md#query-counts). ### Timing guidelines for migrations diff --git a/doc/development/diffs.md b/doc/development/diffs.md index fba8eda0408..52ba89a4d6e 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -14,12 +14,20 @@ We rely on different sources to present diffs. These include: ## Deep Dive + + In January 2019, Oswaldo Ferreira hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) on GitLab Diffs and Commenting on Diffs -functionality to share his domain specific knowledge with anyone who may work in this part of the -codebase in the future. You can find the [recording on YouTube](https://www.youtube.com/watch?v=K6G3gMcFyek), -and the slides on [Google Slides](https://docs.google.com/presentation/d/1bGutFH2AT3bxOPZuLMGl1ANWHqFnrxwQwjiwAZkF-TU/edit) -and in [PDF](https://gitlab.com/gitlab-org/create-stage/uploads/b5ad2f336e0afcfe0f99db0af0ccc71a/). +functionality to share domain-specific knowledge with anyone who may work in this part of the +codebase in the future: + + + +- + [Recording on YouTube](https://www.youtube.com/watch?v=K6G3gMcFyek) +- Slides on [Google Slides](https://docs.google.com/presentation/d/1bGutFH2AT3bxOPZuLMGl1ANWHqFnrxwQwjiwAZkF-TU/edit) +- [PDF slides](https://gitlab.com/gitlab-org/create-stage/uploads/b5ad2f336e0afcfe0f99db0af0ccc71a/) + Everything covered in this deep dive was accurate as of GitLab 11.7, and while specific details may have changed since then, it should still serve as a good introduction. @@ -125,7 +133,7 @@ Gitaly only returns `Diff.Collapsed` (RPC) when surpassing collection limits. #### Not expandable patches (too large) The patch not be rendered if it's larger than `ApplicationSettings#diff_max_patch_bytes`. -Users see a `This source diff could not be displayed because it is too large` message. +Users see a `Changes are too large to be shown.` message and a button to view only that file in that commit. ```ruby Commit::DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 @@ -180,8 +188,8 @@ has been introduced. One of the key challenges to deal with when working on merge ref diffs are merge conflicts. If the target and source branch contains a merge conflict, the branches -cannot be automatically merged. The [recording on -YouTube](https://www.youtube.com/watch?v=GFXIFA4ZuZw&feature=youtu.be&ab_channel=GitLabUnfiltered) +cannot be automatically merged. The + [recording on YouTube](https://www.youtube.com/watch?v=GFXIFA4ZuZw&feature=youtu.be&ab_channel=GitLabUnfiltered) is a quick introduction to the problem and the motivation behind the [epic](https://gitlab.com/groups/gitlab-org/-/epics/854). In 13.5 a solution for both-modified merge diff --git a/doc/development/directory_structure.md b/doc/development/directory_structure.md new file mode 100644 index 00000000000..c2329feb941 --- /dev/null +++ b/doc/development/directory_structure.md @@ -0,0 +1,36 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Backend directory structure + +## Use namespaces to define bounded contexts + +A healthy application is divided into macro and sub components that represent the contexts at play, +whether they are related to business domain or infrastructure code. + +As GitLab code has so many features and components it's hard to see what contexts are involved. +We should expect any class to be defined inside a module/namespace that represents the contexts where it operates. + +When we namespace classes inside their domain: + +- Similar terminology becomes unambiguous as the domain clarifies the meaning: + For example, `MergeRequests::Diff` and `Notes::Diff`. +- Top-level namespaces could be associated to one or more groups identified as domain experts. +- We can better identify the interactions and coupling between components. + For example, several classes inside `MergeRequests::` domain interact more with `Ci::` + domain and less with `ImportExport::`. + +```ruby +# bad +class MyClass +end + +# good +module MyDomain + class MyClass + end +end +``` diff --git a/doc/development/distributed_tracing.md b/doc/development/distributed_tracing.md index 9228609aae9..eb20e721e46 100644 --- a/doc/development/distributed_tracing.md +++ b/doc/development/distributed_tracing.md @@ -4,7 +4,7 @@ group: Health info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Distributed Tracing - development guidelines +# Distributed Tracing - development guidelines **(FREE)** GitLab is instrumented for distributed tracing. Distributed Tracing in GitLab is currently considered **experimental**, as it has not yet been tested at scale on GitLab.com. diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md deleted file mode 100644 index 1595a841ade..00000000000 --- a/doc/development/doc_styleguide.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -redirect_to: 'documentation/styleguide.md' ---- - -This document was moved to [another location](documentation/styleguide.md). - - - diff --git a/doc/development/documentation/feature-change-workflow.md b/doc/development/documentation/feature-change-workflow.md deleted file mode 100644 index 78e5510ffca..00000000000 --- a/doc/development/documentation/feature-change-workflow.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -redirect_to: 'workflow.md' ---- - -This document was moved to [another location](workflow.md). - - - diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index 7547ec59fb2..c9c291abd2c 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -30,7 +30,7 @@ See how to document them below, according to the state of the flag: - [Features that can be enabled or disabled for a single project](#features-enabled-by-project). - [Features with the feature flag removed](#features-with-flag-removed). -The [`**(CORE ONLY)**`](styleguide/index.md#product-tier-badges) badge or equivalent for +The [`**(FREE SELF)**`](styleguide/index.md#product-tier-badges) badge or equivalent for the feature's tier should be added to the line and heading that refers to enabling/disabling feature flags as Admin access is required to do so, therefore, it indicates that it cannot be done by regular users of GitLab.com. @@ -61,7 +61,7 @@ be enabled for a single project, and is not ready for production use: > - It's [deployed behind a feature flag](/user/feature_flags.md), disabled by default. > - It's disabled on GitLab.com. > - It's not recommended for production use. -> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#anchor-to-section). **(CORE ONLY)** +> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#anchor-to-section). **(FREE SELF)** WARNING: This feature might not be available to you. Check the **version history** note above for details. @@ -70,7 +70,7 @@ This feature might not be available to you. Check the **version history** note a -### Enable or disable **(CORE ONLY)** +### Enable or disable **(FREE SELF)** is under development and not ready for production use. It is deployed behind a feature flag that is **disabled by default**. @@ -91,7 +91,7 @@ Feature.disable(:) ```` Adjust the blurb according to the state of the feature you're documenting. -Replace ``, `**(CORE ONLY)**`, ``, and +Replace ``, `**(FREE SELF)**`, ``, and ``, and `#anchor-to-section` accordingly. ### Features that became enabled by default @@ -120,7 +120,7 @@ use: > - [Became enabled by default](link-to-issue) on GitLab 12.1. > - It's enabled on GitLab.com. > - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** +> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(FREE SELF)** WARNING: This feature might not be available to you. Check the **version history** note above for details. @@ -129,7 +129,7 @@ This feature might not be available to you. Check the **version history** note a -### Enable or disable **(CORE ONLY)** +### Enable or disable **(FREE SELF)** is under development but ready for production use. It is deployed behind a feature flag that is **enabled by default**. @@ -150,7 +150,7 @@ Feature.disable(:) ```` Adjust the blurb according to the state of the feature you're documenting. -Replace ``, `**(CORE ONLY)**`, ``, +Replace ``, `**(FREE SELF)**`, ``, ``, and `#anchor-to-section` accordingly. ### Features directly enabled by default @@ -176,7 +176,7 @@ cannot be enabled for a single project, and is ready for production use: > - It's [deployed behind a feature flag](/user/feature_flags.md), enabled by default. > - It's enabled on GitLab.com. > - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** +> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(FREE SELF)** WARNING: This feature might not be available to you. Check the **version history** note above for details. @@ -185,7 +185,7 @@ This feature might not be available to you. Check the **version history** note a -### Enable or disable **(CORE ONLY)** +### Enable or disable **(FREE SELF)** is under development but ready for production use. It is deployed behind a feature flag that is **enabled by default**. @@ -206,7 +206,7 @@ Feature.disable(:) ```` Adjust the blurb according to the state of the feature you're documenting. -Replace ``, `**(CORE ONLY)**`, ``, +Replace ``, `**(FREE SELF)**`, ``, ``, and `#anchor-to-section` accordingly. ### Features enabled by project @@ -249,7 +249,7 @@ be enabled by project, and is ready for production use: > - It's enabled on GitLab.com. > - It can be enabled or disabled for a single project. > - It's recommended for production use. -> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(CORE ONLY)** +> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#anchor-to-section). **(FREE SELF)** WARNING: This feature might not be available to you. Check the **version history** note above for details. @@ -258,14 +258,14 @@ This feature might not be available to you. Check the **version history** note a -### Enable or disable **(CORE ONLY)** +### Enable or disable **(FREE SELF)** is under development but ready for production use. It is deployed behind a feature flag that is **enabled by default**. [GitLab administrators with access to the GitLab Rails console](/administration/feature_flags.md) can opt to disable it. -To enabled it: +To enable it: ```ruby # For the instance @@ -285,7 +285,7 @@ Feature.disable(:, Project.find()) ```` Adjust the blurb according to the state of the feature you're documenting. -Replace ``, `**(CORE ONLY)**`, ``, +Replace ``, `**(FREE SELF)**`, ``, ``, and `#anchor-to-section` accordingly. ### Features with flag removed diff --git a/doc/development/documentation/improvement-workflow.md b/doc/development/documentation/improvement-workflow.md deleted file mode 100644 index 78e5510ffca..00000000000 --- a/doc/development/documentation/improvement-workflow.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -redirect_to: 'workflow.md' ---- - -This document was moved to [another location](workflow.md). - - - diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index 55f5d43b175..53e7ba35831 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -12,7 +12,7 @@ The GitLab documentation is [intended as the single source of truth (SSOT)](http In addition to this page, the following resources can help you craft and contribute to documentation: - [Style Guide](styleguide/index.md) - What belongs in the docs, language guidelines, Markdown standards to follow, links, and more. -- [Structure and template](structure.md) - Learn the typical parts of a doc page and how to write each one. +- [Topic type template](structure.md) - Learn about the different types of topics. - [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 is built. @@ -96,18 +96,18 @@ belongs to, as well as an information block as described below: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments ``` -For example, the following metadata would be at the beginning of a product -documentation page whose content is primarily associated with the Audit Events -feature: +For example: ```yaml --- -stage: Monitor -group: APM +stage: Example Stage +group: Example Group info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- ``` +If you need help determining the correct stage, read [Ask for help](workflow.md#ask-for-help). + ### Document type metadata Originally discussed in [this epic](https://gitlab.com/groups/gitlab-org/-/epics/1280), @@ -161,73 +161,69 @@ Nanoc layout), which is displayed at the top of the page if defined: ## Move or rename a page -Moving or renaming a document is the same as changing its location. -Be sure to assign a technical writer to any MR that renames or moves a page. Technical -Writers can help with any questions and can review your change. +Moving or renaming a document is the same as changing its location. Be sure to +assign a technical writer to any merge request that renames or moves a page. +Technical Writers can help with any questions and can review your change. -When moving or renaming a page, you must redirect browsers to the new page. This -ensures users find the new page, and have the opportunity to update their bookmarks. +When moving or renaming a page, you must redirect browsers to the new page. +This ensures users find the new page, and have the opportunity to update their +bookmarks. There are two types of redirects: -- Redirect files added into the docs themselves, for users who view the docs in `/help` - on self-managed instances. For example, [`/help` on GitLab.com](https://gitlab.com/help). -- Redirects in a [`_redirects`](../../user/project/pages/redirects.md) file, for users - who view the docs on . - -To add a redirect: - -1. In an MR in one of the internal docs projects (`gitlab`, `gitlab-runner`, `omnibus-gitlab` - or `charts`): - 1. Move or rename the doc, but do not delete the old doc. - 1. In the old doc, add the redirect code for `/help`. Use the following template exactly, - and only change the links and date. Use relative paths and `.md` for a redirect - to another docs page. Use the full URL to redirect to a different project or site: - - ```markdown - --- - redirect_to: '../path/to/file/index.md' - --- - - This document was moved to [another location](../path/to/file/index.md). - - - - ``` +- Redirect codes added into the documentation files themselves, for users who + view the docs in `/help` on self-managed instances. For example, + [`/help` on GitLab.com](https://gitlab.com/help). +- [GitLab Pages redirects](../../user/project/pages/redirects.md), + for users who view the docs on [`docs.gitlab.com`](https://docs.gitlab.com). - Redirect files linking to docs in any of the 4 internal docs projects can be - removed after 3 months. Redirect files linking to external sites can be removed - after 1 year. +The Technical Writing team manages the [process](https://gitlab.com/gitlab-org/technical-writing/-/blob/master/.gitlab/issue_templates/tw-monthly-tasks.md) +to regularly update the [`redirects.yaml`](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/content/_data/redirects.yaml) +file. - 1. If the document being moved has any Disqus comments on it, follow the steps - described in [Redirections for pages with Disqus comments](#redirections-for-pages-with-disqus-comments). - 1. If a documentation page you're removing includes images that aren't used - with any other documentation pages, be sure to use your MR to delete - those images from the repository. - 1. Assign the MR to a technical writer for review and merge. -1. If the redirect is to one of the 4 internal docs projects (not an external URL), - create an MR in [`gitlab-docs`](https://gitlab.com/gitlab-org/gitlab-docs): - 1. Update [`_redirects`](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/content/_redirects) - with one redirect entry for each renamed or moved file. This code works for - links only: - - ```plaintext - /ee/path/to/old_file.html /ee/path/to/new_file 302 # To be removed after YYYY-MM-DD - ``` +To add a redirect: - The path must start with the internal project directory `/ee` for `gitlab`, - `/gitlab-runner`, `/omnibus-gitlab` or `charts`, and must end with `.html`. +1. Create a merge request in one of the internal docs projects (`gitlab`, + `gitlab-runner`, `omnibus-gitlab`, or `charts`), depending on the location of + the file that's being moved, renamed, or removed. +1. To move or rename the documentation file, create a new file with the new + name or location, but don't delete the existing documentation file. +1. In the original documentation file, add the redirect code for + `/help`. Use the following template exactly, and change only the links and + date. Use relative paths and `.md` for a redirect to another documentation + page. Use the full URL (with `https://`) to redirect to a different project or + site: + + ```markdown + --- + redirect_to: '../path/to/file/index.md' + --- + + This document was moved to [another location](../path/to/file/index.md). + + + + ``` - `_redirects` entries can be removed after one year. + Redirect files linking to docs in any of the internal documentations projects + are removed after three months. Redirect files linking to external sites are + removed after one year. -1. Search for links to the old file. You must find and update all links to the old file: +1. If the documentation page being moved has any Disqus comments, follow the steps + described in [Redirections for pages with Disqus comments](#redirections-for-pages-with-disqus-comments). +1. If a documentation page you're removing includes images that aren't used + with any other documentation pages, be sure to use your merge request to delete + those images from the repository. +1. Assign the merge request to a technical writer for review and merge. +1. Search for links to the original documentation file. You must find and update all + links that point to the original documentation file: - In , search for full URLs: `grep -r "docs.gitlab.com/ee/path/to/file.html" .` - In , search the navigation bar configuration files for the path with `.html`: `grep -r "path/to/file.html" .` - - In any of the 4 internal projects. This includes searching for links in the docs + - In any of the four internal projects. This includes searching for links in the docs and codebase. Search for all variations, including full URL and just the path. In macOS for example, go to the root directory of the `gitlab` project and run: @@ -238,8 +234,8 @@ To add a redirect: grep -r "path/to/file" . ``` - You may need to try variations of relative links as well, such as `../path/to/file` - or even `../file` to find every case. + You may need to try variations of relative links, such as `../path/to/file` or + `../file` to find every case. ### Redirections for pages with Disqus comments @@ -306,68 +302,51 @@ with GitLab 11.4. Meaning, it's available only with `/help` from GitLab ### Linking to `/help` -When you're building a new feature, you may need to link the documentation -from GitLab, the application. This is normally done in files inside the -`app/views/` directory with the help of the `help_page_path` helper method. - -In its simplest form, the HAML code to generate a link to the `/help` page is: - -```haml -= link_to 'Help page', help_page_path('user/permissions') -``` - -The `help_page_path` contains the path to the document you want to link to with -the following conventions: - -- it is relative to the `doc/` directory in the GitLab repository -- the `.md` extension must be omitted -- it must not end with a slash (`/`) +When you're building a new feature, you may need to link to the documentation +from the GitLab application. This is normally done in files inside the +`app/views/` directory, with the help of the `help_page_path` helper method. -Below are some special cases where should be used depending on the context. -You can combine one or more of the following: +The `help_page_path` contains the path to the document you want to link to, +with the following conventions: -1. **Linking to an anchor link.** Use `anchor` as part of the `help_page_path` - method: +- It's relative to the `doc/` directory in the GitLab repository. +- It omits the `.md` extension. +- It doesn't end with a slash (`/`). - ```haml - = link_to 'Help page', help_page_path('user/permissions', anchor: 'anchor-link') - ``` +The help text follows the [Pajamas guidelines](https://design.gitlab.com/usability/helping-users/#formatting-help-content). -1. **Opening links in a new tab.** This should be the default behavior: +Use the following special cases depending on the context, ensuring all links +are inside `_()` so they can be translated: - ```haml - = link_to 'Help page', help_page_path('user/permissions'), target: '_blank' - ``` +- Linking to a doc page. In its most basic form, the HAML code to generate a + link to the `/help` page is: -1. **Using a question icon.** Usually used in settings where a long - description cannot be used, like near checkboxes. You can basically use - any GitLab SVG icon, but prefer the `question-o`: - - ```haml - = link_to sprite_icon('question-o'), help_page_path('user/permissions') - ``` + ```haml + = link_to _('Learn more.'), help_page_path('user/permissions'), target: '_blank', rel: 'noopener noreferrer' + ``` -1. **Using a button link.** Useful in places where text would be out of context - with the rest of the page layout: +- Linking to an anchor link. Use `anchor` as part of the `help_page_path` + method: - ```haml - = link_to 'Help page', help_page_path('user/permissions'), class: 'btn btn-info' - ``` + ```haml + = link_to _('Learn more.'), help_page_path('user/permissions', anchor: 'anchor-link'), target: '_blank', rel: 'noopener noreferrer' + ``` -1. **Using links inline of some text.** +- Using links inline of some text. First, define the link, and then use it. In + this example, `link_start` is the name of the variable that contains the + link: - ```haml - Description to #{link_to 'Help page', help_page_path('user/permissions')}. - ``` + ```haml + - link_start = ''.html_safe % { url: help_page_path('user/permissions') } + %p= _("This is a text describing the option/feature in a sentence. %{link_start}Learn more.%{link_end}").html_safe % { link_start: link_start, link_end: ''.html_safe } + ``` -1. **Adding a period at the end of the sentence.** Useful when you don't want - the period to be part of the link: +- Using a button link. Useful in places where text would be out of context with + the rest of the page layout: - ```haml - = succeed '.' do - Learn more in the - = link_to 'Help page', help_page_path('user/permissions') - ``` + ```haml + = link_to _('Learn more.'), help_page_path('user/permissions'), class: 'btn btn-info', target: '_blank', rel: 'noopener noreferrer' + ``` #### Linking to `/help` in JavaScript @@ -514,7 +493,7 @@ To run the tool on an existing screenshot generator, take the following steps: 1. Set up the [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/gitlab_docs.md). 1. Navigate to the subdirectory with your cloned GitLab repository, typically `gdk/gitlab`. 1. Make sure that your GDK database is fully migrated: `bin/rake db:migrate RAILS_ENV=development`. -1. Install pngquant, see the tool website for more information: [`pngquant`](https://pngquant.org/) +1. Install `pngquant`, see the tool website for more information: [`pngquant`](https://pngquant.org/) 1. Run `scripts/docs_screenshots.rb spec/docs_screenshots/.rb `. 1. Identify the location of the screenshots, based on the `gitlab/doc` location defined by the `it` parameter in your script. 1. Commit the newly created screenshots. diff --git a/doc/development/documentation/restful_api_styleguide.md b/doc/development/documentation/restful_api_styleguide.md index c8c48e71b48..e05f6760ff1 100644 --- a/doc/development/documentation/restful_api_styleguide.md +++ b/doc/development/documentation/restful_api_styleguide.md @@ -165,7 +165,7 @@ curl --request POST \ ### Post data using form-data -Instead of using JSON or urlencode you can use multipart/form-data which +Instead of using JSON or URL-encoding data, you can use `multipart/form-data` which properly handles data encoding: ```shell diff --git a/doc/development/documentation/site_architecture/global_nav.md b/doc/development/documentation/site_architecture/global_nav.md index fcf4662502f..f66b0543ad1 100644 --- a/doc/development/documentation/site_architecture/global_nav.md +++ b/doc/development/documentation/site_architecture/global_nav.md @@ -283,8 +283,8 @@ and the following syntax rules. - As convention, always wrap URLs in single quotes `'url'`. - Always use relative paths against the home of CE and EE. Examples: - For `https://docs.gitlab.com/ee/README.html`, the relative URL is `README.html`. - - For `https://docs.gitlab.com/ee/user/project/cycle_analytics.html`, the relative - URL is `user/project/cycle_analytics.html`. + - For `https://docs.gitlab.com/ee/user/analytics/value_stream_analytics.md`, the relative + URL is `user/analytics/value_stream_analytics.html`. - For `README.html` files, add the complete path `path/to/README.html`. - For `index.html` files, use the clean (canonical) URL: `path/to/`. - For EE-only docs, use the same relative path, but add the attribute `ee_only: true` below @@ -328,7 +328,7 @@ There are three main considerations on the logic built for the nav: - `https://docs.gitlab.com/*` - [EE-only](#ee-only-docs): documentation only available in `/ee/`, not on `/ce/`, e.g.: - `https://docs.gitlab.com/ee/user/group/epics/` - - `https://docs.gitlab.com/ee/user/project/security_dashboard.html` + - `https://docs.gitlab.com/ee/user/application_security/security_dashboard/index.html` - [Default URL](#default-url): between CE and EE docs, the default is `ee`, therefore, all docs should link to `/ee/` unless if on `/ce/` linking internally to `ce`. diff --git a/doc/development/documentation/site_architecture/index.md b/doc/development/documentation/site_architecture/index.md index be25a083948..92fd17f9d3e 100644 --- a/doc/development/documentation/site_architecture/index.md +++ b/doc/development/documentation/site_architecture/index.md @@ -114,7 +114,7 @@ located in the [Dockerfiles directory](https://gitlab.com/gitlab-org/gitlab-docs If you need to rebuild the Docker images immediately (must have maintainer level permissions): WARNING: -If you change the dockerfile configuration and rebuild the images, you can break the master +If you change the Dockerfile configuration and rebuild the images, you can break the master pipeline in the main `gitlab` repository as well as in `gitlab-docs`. Create an image with a different name first and test it to ensure you do not break the pipelines. diff --git a/doc/development/documentation/structure.md b/doc/development/documentation/structure.md index fd0c29f0fc1..65949d5b5f5 100644 --- a/doc/development/documentation/structure.md +++ b/doc/development/documentation/structure.md @@ -5,167 +5,165 @@ info: To determine the technical writer assigned to the Stage/Group associated w description: What to include in GitLab documentation pages. --- -# Documentation structure and template +# Documentation topic types -Use these standards to contribute content to the GitLab documentation. +At GitLab, we have not traditionally used topic types. However, we are starting to +move in this direction, and we now use four topic types: -Before getting started, familiarize yourself with [Documentation guidelines for GitLab](index.md) -and the [Documentation Style Guide](styleguide/index.md). +- [Concept](#concept) +- [Task](#task) +- [Reference](#reference) +- [Troubleshooting](#troubleshooting) -## Components of a documentation page +Each page contains multiple topic types. For example, +a page with the title `Pipelines`, which is generated from a file called `index.md`, +can include a concept and multiple task and reference topics. -Most pages are dedicated to a specific GitLab feature or to a use case that -involves one or more features, potentially in conjunction with third-party tools. +GitLab also uses high-level landing pages. -In general, each topic should include the following content, in this sequence: +## Landing pages -- *Metadata*: Information about the stage, group, and how to find the technical - writer for the topic. This information isn't visible in the published help. -- *Title*: A top-level heading with the feature or use case name. Choose a term - that defines the functionality and use the same term in all the resources - where the feature is mentioned. -- *Introduction*: In a few sentences beneath the title, describe what the - feature or topic is, what it does, and in what context it should be used. -- *Use cases*: Describe real user scenarios. -- *Prerequisites*: Describe the software, configuration, account, permissions, - or knowledge required to use this functionality. -- *Tasks*: Present detailed step-by-step instructions on how to use the feature. -- *Troubleshooting*: List errors and how to address them. Recommended but not - required. +Landing pages are topics that group other topics and help a user to navigate a section. -You can include additional subsections, as appropriate, such as *How it Works*, -or *Architecture*. You can also include other logical divisions, such as -pre-deployment and post-deployment tasks. +Users who are using the in-product help do not have a left nav, +and need these topics to navigate the documentation. -## Template for new docs +These topics can also help other users find the most important topics +in a section. -Follow the [folder structure and filename guidelines](styleguide/index.md#folder-structure-overview) -and create a new topic by using this template: +Landing page topics should be in this format: ```markdown - ---- -description: "Short document description." # Up to ~200 chars long. This information is displayed -in Google Search snippets. It may help to write the page intro first, and then reuse it here. -stage: Add the stage name here -group: Add the group name here -info: To determine the technical writer assigned to the Stage/Group associated with this page, -see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments ---- +# Title (a noun, like "CI/CD or "Analytics") -# Feature or Use Case Name **[TIER]** (1) - +Brief introduction to the concept or product area. +Include the reason why someone would use this thing. + +- Bulleted list of important related topics. +- These links are needed because users of in-product help do not have left navigation. +``` - -> [Introduced](link_to_issue_or_mr) in GitLab (Tier) X.Y (2). +## Concept -Write a description of the feature or use case. This introduction should answer -these questions: +A concept topic introduces a single feature or concept. -- What is this feature or use case? -- Who is it for? -- What is the context in which it is used and are there any prerequisites or - requirements? -- What can the audience do with this? (Be sure to consider all applicable - audiences, such as GitLab admin and developer-user.) -- What are the benefits of using this over any existing alternatives? +A concept should answer the questions: -You can reuse this content, or part of it, for the front matter's `description` -at the top of this file. +- What is this? +- Why would I use it? -## Use cases +Think of everything someone might want to know if they’ve never heard of this topic before. -Describe common use cases, typically in bulleted form. Include real-life examples -for each. +Don’t tell them **how** to do this thing. Tell them **what it is**. -If the page itself is dedicated to a use case, this section usually includes more -specific scenarios for use (for example, variations on the main use case), but if -that's not applicable, you can omit this section. +If you start describing another topic, start a new concept and link to it. + +Concept topics should be in this format: + +```markdown +# Title (a noun, like "Widgets") -Examples of use cases on feature pages: +A paragraph that explains what this thing is. -- CE and EE: [Issues](../../user/project/issues/index.md#use-cases) -- CE and EE: [Merge Requests](../../user/project/merge_requests/index.md) -- EE-only: [Geo](../../administration/geo/index.md) -- EE-only: [Jenkins integration](../../integration/jenkins.md) +Another paragraph that explains what this thing is. -## Prerequisites +Remember, if you start to describe about another concept, stop yourself. +Each concept topic should be about one concept only. +``` -State any prerequisites for using the feature. These might include: +## Task -- Technical prereqs (for example, an account on a third-party service, an amount - of storage space, or prior configuration of another feature) -- Prerequisite knowledge (for example, familiarity with certain GitLab features - or other products and technologies). +A task topic gives instructions for how to complete a procedure. -Link each one to an appropriate place for more information. +Task topics should be in this format: -## Tasks +```markdown +# Title (starts with an active verb, like "Create a widget" or "Delete a widget") -Each topic should help users accomplish a specific task. +Do this task when you want to... -The heading should: +Prerequisites (optional): -- Describe the task and start with a verb. For example, `Create a package` or - `Configure a pipeline`. -- Be short and descriptive (up to ~50 chars). -- Start from an `h2` (`##`), then go over `h3`, `h4`, `h5`, and `h6` as needed. - Never skip a hierarchy level (like `h2` > `h4`). It breaks the table of - contents and can affect the breadcrumbs. +- Thing 1 +- Thing 2 +- Thing 3 -Bigger tasks can have subsections that explain specific phases of the process. +To do this task: -Include example code or configurations when needed. Use Markdown to wrap code -blocks with [syntax highlighting](../../user/markdown.md#colored-code-and-syntax-highlighting). +1. Location then action. (Go to this menu, then select this item.) +1. Another step. +1. Another step. -Example topic: +Task result (optional). Next steps (optional). +``` -## Create a teddy bear +Here is an example. -Create a teddy bear when you need something to hug. (Include the reason why you -might do the task.) +```markdown +# Create an issue -To create a teddy bear: +Create an issue when you want to track bugs or future work. -1. Go to **Settings > CI/CD**. -1. Expand **This** and click **This**. -1. Do another step. +Prerequisites: -The teddy bear is now in the kitchen, in the cupboard above the sink. _(This is the result.)_ +- A minimum of Contributor access to a project in GitLab. -You can retrieve the teddy bear and put it on the couch with the other animals. _(These are next steps.)_ +To create an issue: -Screenshots are not necessary. They are difficult to keep up-to-date and can -clutter the page. +1. Go to **Issues > List**. +1. In the top right, click **New issue**. +1. Complete the fields. (If you have a reference topic that lists each field, link to it here.) +1. Click **Submit issue**. - +A reference topic provides information in an easily-scannable format, +like a table or list. It's similar to a dictionary or encyclopedia entry. ---- +```markdown +# Title (a noun, like "Pipeline settings" or "Administrator options") -Notes: +Introductory sentence. -- (1): Apply the [tier badges](styleguide/index.md#product-badges) accordingly. -- (2): Apply the correct format for the - [GitLab version that introduces the feature](styleguide/index.md#gitlab-versions-and-tiers). +| Setting | Description | +|---------|-------------| +| **Name** | Descriptive sentence about the setting. | ``` -## Help and feedback section +## Troubleshooting + +Troubleshooting topics can be one of two categories: + +- **Troubleshooting task.** This topic is written the same as a [standard task topic](#task). + For example, "Run debug tools" or "Verify syntax." +- **Troubleshooting reference.** This topic has a specific format. + +Troubleshooting reference topics should be in this format: + +```markdown +# Title (the error message or a description of it) + +You might get an error that states . + +This issue occurs when... + +The workaround is... +``` + +## Other information on a topic + +Topics include other information. + +For example: + +- Each topic must have a [tier badge](styleguide/index.md#product-tier-badges). +- New topics must have information about the + [GitLab version where the feature was introduced](styleguide/index.md#where-to-put-version-text). + +### Help and feedback section This section ([introduced](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/319) in GitLab 11.4) is displayed at the end of each document and can be omitted by adding a key into @@ -180,7 +178,7 @@ feedback: false The default is to leave it there. If you want to omit it from a document, you must check with a technical writer before doing so. -### Disqus +#### Disqus We also have integrated the docs site with Disqus (introduced by [!151](https://gitlab.com/gitlab-org/gitlab-docs/-/merge_requests/151)), @@ -206,7 +204,7 @@ The click events in the feedback section are tracked with Google Tag Manager. The conversions can be viewed on Google Analytics by navigating to **Behavior > Events > Top events > docs**. -## Guidelines for good practices +### Guidelines for good practices > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36576/) in GitLab 13.2 as GitLab Development documentation. diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index a12c2740083..ade0b89a92c 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -3,3 +3,6 @@ redirect_to: 'styleguide/index.md' --- This document was moved to [another location](styleguide/index.md). + + + diff --git a/doc/development/documentation/styleguide/img/tier_badge.png b/doc/development/documentation/styleguide/img/tier_badge.png index 674d869da9b..5fc38e08172 100644 Binary files a/doc/development/documentation/styleguide/img/tier_badge.png and b/doc/development/documentation/styleguide/img/tier_badge.png differ diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index bba94c7de7e..7737aa58506 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -7,34 +7,25 @@ description: 'Writing styles, markup, formatting, and other standards for GitLab # Documentation Style Guide -This document defines the standards for GitLab documentation. +This document defines the standards for GitLab documentation, including grammar, formatting, word use, and more. -For broader information about the documentation, see the [Documentation guidelines](../index.md). +For style questions, mention `@tw-style` in an issue or merge request. If you have access to the GitLab Slack workspace, +use the `#docs-processes` channel. -For guidelines specific to text in the GitLab interface, see the Pajamas [Content](https://design.gitlab.com/content/error-messages/) section. +In addition to this page, the following resources can help you craft and contribute to documentation: -For information on how to validate styles locally or by using GitLab CI/CD, see [Testing](../testing.md). - -Use this guide for standards on grammar, formatting, word usage, and more. - -You can also view a list of [recent updates to this guide](https://gitlab.com/dashboard/merge_requests?scope=all&utf8=%E2%9C%93&state=merged&label_name[]=tw-style¬[label_name][]=docs%3A%3Afix). - -If you can't find what you need: - -- View the GitLab Handbook for [writing style guidelines](https://about.gitlab.com/handbook/communication/#writing-style-guidelines) that apply to all GitLab content. -- Refer to: - - - [Microsoft Style Guide](https://docs.microsoft.com/en-us/style-guide/welcome/). - - [Google Developer Documentation Style Guide](https://developers.google.com/style). - -If you have questions about style, mention `@tw-style` in an issue or merge request, or, if you have access to the GitLab Slack workspace, use `#docs-process`. +- [Doc contribution guidelines](../index.md) +- [Doc style and consistency testing](../testing.md) +- [UI text guidelines](https://design.gitlab.com/content/error-messages/) +- [GitLab Handbook style guidelines](https://about.gitlab.com/handbook/communication/#writing-style-guidelines) +- [Microsoft Style Guide](https://docs.microsoft.com/en-us/style-guide/welcome/) +- [Google Developer Documentation Style Guide](https://developers.google.com/style) +- [Recent updates to this guide](https://gitlab.com/dashboard/merge_requests?scope=all&utf8=%E2%9C%93&state=merged&label_name[]=tw-style¬[label_name][]=docs%3A%3Afix) ## Documentation is the single source of truth (SSOT) -### Why a single source of truth - -The documentation of GitLab products and features is the SSOT for all -information related to implementation, usage, and troubleshooting. It evolves +The GitLab documentation is the SSOT for all +information related to GitLab implementation, usage, and troubleshooting. It evolves continuously, in keeping with new products and features, and with improvements for clarity, accuracy, and completeness. @@ -44,7 +35,7 @@ about GitLab products. It also informs decisions about the kinds of content we include in our documentation. -### All information +### The documentation includes all information Include problem-solving actions that may address rare cases or be considered _risky_, but provide proper context through fully-detailed @@ -54,10 +45,13 @@ If you think you have found an exception to this rule, contact the Technical Writing team. GitLab adds all troubleshooting information to the documentation, no matter how -unlikely a user is to encounter a situation. For the [Troubleshooting sections](#troubleshooting), -people in GitLab Support can merge additions themselves. +unlikely a user is to encounter a situation. + +GitLab Support maintains their own +[troubleshooting content](../../../administration/index.md#support-team-docs) +in the GitLab documentation. -### All media types +### The documentation includes all media types Include any media types/sources if the content is relevant to readers. You can freely include or link presentations, diagrams, and videos. No matter who @@ -71,48 +65,33 @@ include it. quotation with the source cited. Typically it is better to either rephrase relevant information in your own words or link out to the other source. -### No special types +### Topic types In the software industry, it is a best practice to organize documentation in -different types. For example, [Divio recommends](https://www.divio.com/blog/documentation/): +different types. For example: -- Tutorials -- How-to guides -- Explanation -- Reference (for example, a glossary) +- Concepts +- Tasks +- Reference +- Troubleshooting -At GitLab, we have so many product changes in our monthly releases that we can't -afford to continuously update multiple types of information. If we have multiple -types, the information becomes outdated. Therefore, we have a -[single template](../structure.md) for documentation. +At GitLab, we have not traditionally used topic types. However, we are starting to +move in this direction, so we can address these issues: -GitLab documentation does not distinguish specific document types. We are open to -reconsidering this policy after the documentation has reached a future stage of -maturity and quality. If you are reading this, then despite our continuous -improvement efforts, that point hasn't been reached. +- **Content is hard to find.** Our docs are comprehensive and include a large amount of + useful information. Topic types create repeatable patterns that make our content easier + to scan and parse. +- **Content is often written from the contributor's point of view.** Our docs + are written by contributors. Topic types (tasks specifically) help put + information into a format that is geared toward helping others, rather than + documenting how a feature was implemented. -### Link instead of summarize +GitLab uses these [topic type templates](../structure.md). -There is a temptation to summarize the information on another page, which -causes the information to live in two places. Instead, link to the single source -of truth and explain why it is important to consume the information. +### Link instead of repeating text -### Organize by topic, not by type - -We organize content by topic, not by type, so it can be located in the -single-source-of-truth (SSOT) section for the subject matter. Top-level audience-type -folders, like `administration`, are exceptions. - -For example, do not create groupings of similar media types. For example: - -- Glossaries. -- FAQs. -- Sets of all articles or videos. - -Such grouping of content by type makes it difficult to browse for the information -you need and difficult to maintain up-to-date content. Instead, organize content -by its subject (for example, everything related to CI goes together) and -cross-link between any related content. +Rather than repeating information from another topic, link to the single source +of truth and explain why it is important. ### Docs-first methodology @@ -127,14 +106,9 @@ of GitLab more efficient. should be to create a merge request (MR) to add this information to the documentation. You can then share the MR to communicate this information. -New information about the future usage or troubleshooting -of GitLab should not be written directly in a forum or other messaging system. -Instead, add it to a documentation merge request, then reference it. Note -that among any other documentation changes, you can either: - -- Add a [Troubleshooting section](#troubleshooting) to a doc if none exists. -- Un-comment and use the placeholder Troubleshooting section included as part of - our [documentation template](../structure.md#template-for-new-docs), if present. +New information that would be useful toward the future usage or troubleshooting +of GitLab should not be written directly in a forum or other messaging system, +but added to a documentation MR and then referenced, as described above. The more we reflexively add information to the documentation, the more the documentation helps others efficiently accomplish tasks and solve problems. @@ -217,8 +191,11 @@ included in backticks. For example: ## Structure -Because we want documentation to be a SSOT, we should [organize by topic, not by -type](#organize-by-topic-not-by-type). +We include concept and task topic types in the same larger topic. + +In general, we have one topic that's a [landing page](../structure.md#landing-pages). +Below that topic in the left nav are individual topics. Each of these include a concept +and multiple related tasks, reference, and troubleshooting topics. ### Folder structure overview @@ -299,7 +276,7 @@ place for it. ### Avoid duplication Do not include the same information in multiple places. -[Link to a single source of truth instead.](#link-instead-of-summarize) +[Link to a single source of truth instead.](#link-instead-of-repeating-text) ### References across documents @@ -389,7 +366,7 @@ by default. Capitalize names of: - GitLab [product tiers](https://about.gitlab.com/pricing/). For example, - GitLab Core and GitLab Ultimate. (Tested in [`BadgeCapitalization.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/BadgeCapitalization.yml).) + GitLab Free and GitLab Ultimate. (Tested in [`BadgeCapitalization.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/BadgeCapitalization.yml).) - Third-party organizations, software, and products. For example, Prometheus, Kubernetes, Git, and The Linux Foundation. - Methods or methodologies. For example, Continuous Integration, @@ -549,6 +526,7 @@ You can use these fake tokens as examples: | scalability | Do not use when talking about increasing GitLab performance for additional users. The words scale or scaling are sometimes acceptable, but references to increasing GitLab performance for additional users should direct readers to the GitLab [reference architectures](../../../administration/reference_architectures/index.md) page. | | simply | Do not use. If the user doesn't find the process to be these things, we lose their trust. | | slashes | Instead of **and/or**, use **or** or another sensible construction. This rule also applies to other slashes, like **follow/unfollow**. Some exceptions (like **CI/CD**) are allowed. | +| subgroup | Use instead of `sub-group`. | | that | Do not use. Example: `the file that you save` can be `the file you save`. | | useful | Do not use. If the user doesn't find the process to be these things, we lose their trust. | | utilize | Do not use. Use **use** instead. It's more succinct and easier for non-native English speakers to understand. | @@ -765,8 +743,6 @@ Items nested in lists should always align with the first character of the list item. In unordered lists (using `-`), this means two spaces for each level of indentation: - - ````markdown - Unordered list item 1 @@ -788,12 +764,8 @@ indentation: ![an image that will nest inside list item 4](image.png) ```` - - For ordered lists, use three spaces for each level of indentation: - - ````markdown 1. Ordered list item 1 @@ -815,8 +787,6 @@ For ordered lists, use three spaces for each level of indentation: ![an image that will nest inside list item 4](image.png) ```` - - You can nest full lists inside other lists using the same rules as above. If you want to mix types, that's also possible, if you don't mix items at the same level: @@ -942,7 +912,7 @@ NOTE: [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39717) in GitLab 13.4, [product badges](#product-tier-badges) used in headings aren't included in the generated anchor links. For example, when you link to -`## This is an example **(CORE)**`, use the anchor `#this-is-an-example`. +`## This is an example **(FREE)**`, use the anchor `#this-is-an-example`. Keep in mind that the GitLab user interface links to many documentation pages and anchor links to take the user to the right spot. When you change @@ -966,8 +936,8 @@ this option. ## Links -Links are important in GitLab documentation. They allow you to [link instead of -summarizing](#link-instead-of-summarize) to help preserve a [single source of truth](#why-a-single-source-of-truth) +Links are important in GitLab documentation. Use links instead of +summarizing to help preserve a [single source of truth](#documentation-is-the-single-source-of-truth-ssot) in GitLab documentation. We include guidance for links in these categories: @@ -1020,7 +990,8 @@ To link to internal documentation: - Use relative links to Markdown files in the same repository. - Do not use absolute URLs or URLs from `docs.gitlab.com`. - Use `../` to navigate to higher-level directories. -- Don't prepend `./` to links to files or directories. +- Don't prepend `./` to links to files or directories. To link to a file in the + same directory or one of its sub-directories, use the syntax `path/to/file.md`. - Don't link relative to root. For example, `/ee/user/gitlab_com/index.md`. Don't: @@ -1045,6 +1016,7 @@ To link to internal documentation: - `../../merge_requests/index.md` - `../../issues/tags.md` - `../../issues/tags.md#stages` + - `issues/tags.md` NOTE: Using the Markdown extension is necessary for the [`/help`](../index.md#gitlab-help) @@ -1345,8 +1317,6 @@ hidden on the documentation site, but is displayed by `/help`. - For regular fenced code blocks, always use a highlighting class corresponding to the language for better readability. Examples: - - ````markdown ```ruby Ruby code @@ -1365,8 +1335,6 @@ hidden on the documentation site, but is displayed by `/help`. ``` ```` - - Syntax highlighting is required for fenced code blocks added to the GitLab documentation. Refer to this table for the most common language classes, or check the [complete list](https://github.com/rouge-ruby/rouge/wiki/List-of-supported-languages-and-lexers) @@ -1671,8 +1639,8 @@ the blockquote to use a bulleted list: If a feature is moved to another tier: ```markdown -> - [Moved]() from GitLab Premium to GitLab Starter in 11.8. -> - [Moved]() from GitLab Starter to GitLab Core in 12.0. +> - [Moved]() from GitLab Ultimate to GitLab Premium in 11.8. +> - [Moved]() from GitLab Premium to GitLab Free in 12.0. ``` If a feature is deprecated, include a link to a replacement (when available): @@ -1779,7 +1747,7 @@ To add a tier badge to a heading, add the relevant [tier badge](#available-produ after the heading text. For example: ```markdown -# Heading title `**(CORE)**` +# Heading title **(FREE)** ``` #### Product tier badges on other content @@ -1787,30 +1755,26 @@ after the heading text. For example: In paragraphs, list names, and table cells, an information icon displays when you add a tier badge. More verbose information displays when a user points to the icon: -- `**(STARTER)**` displays as **(STARTER)** -- `**(STARTER ONLY)**` displays as **(STARTER ONLY)** -- `**(SILVER ONLY)**` displays as **(SILVER ONLY)** +- `**(FREE)**` displays as **(FREE)** +- `**(FREE SELF)**` displays as **(FREE SELF)** +- `**(FREE SAAS)**` displays as **(FREE SAAS)** -The `**(STARTER)**` generates a `span` element to trigger the -badges and tooltips (``). When the keyword -_only_ is added, the corresponding GitLab.com badge isn't displayed. +The `**(FREE)**` generates a `span` element to trigger the +badges and tooltips (``). #### Available product tier badges -| Tier in which feature is available | Tier badge | -|:-----------------------------------------------------------------------|:----------------------| -| GitLab Core and GitLab.com Free, and their higher tiers | `**(CORE)**` | -| GitLab Starter and GitLab.com Bronze, and their higher tiers | `**(STARTER)**` | -| GitLab Premium and GitLab.com Silver, and their higher tiers | `**(PREMIUM)**` | -| GitLab Ultimate and GitLab.com Gold | `**(ULTIMATE)**` | -| _Only_ GitLab Core and higher tiers (no GitLab.com-based tiers) | `**(CORE ONLY)**` | -| _Only_ GitLab Starter and higher tiers (no GitLab.com-based tiers) | `**(STARTER ONLY)**` | -| _Only_ GitLab Premium and higher tiers (no GitLab.com-based tiers) | `**(PREMIUM ONLY)**` | -| _Only_ GitLab Ultimate (no GitLab.com-based tiers) | `**(ULTIMATE ONLY)**` | -| _Only_ GitLab.com Free and higher tiers (no self-managed instances) | `**(FREE ONLY)**` | -| _Only_ GitLab.com Bronze and higher tiers (no self-managed instances) | `**(BRONZE ONLY)**` | -| _Only_ GitLab.com Silver and higher tiers (no self-managed instances) | `**(SILVER ONLY)**` | -| _Only_ GitLab.com Gold (no self-managed instances) | `**(GOLD ONLY)**` | +| Tier in which feature is available | Tier badge | +|:--------------------------------------------------------------------------|:----------------------| +| GitLab Free self-managed and SaaS, and higher tiers | `**(FREE)**` | +| GitLab Premium self-managed and SaaS, and their higher tiers | `**(PREMIUM)**` | +| GitLab Ultimate self-managed and SaaS | `**(ULTIMATE)**` | +| _Only_ GitLab Free self-managed and higher tiers (no SaaS-based tiers) | `**(FREE SELF)**` | +| _Only_ GitLab Premium self-managed and higher tiers (no SaaS-based tiers) | `**(PREMIUM SELF)**` | +| _Only_ GitLab Ultimate self-managed (no SaaS-based tiers) | `**(ULTIMATE SELF)**` | +| _Only_ GitLab Free SaaS and higher tiers (no self-managed instances) | `**(FREE SAAS)**` | +| _Only_ GitLab Premium SaaS and higher tiers (no self-managed instances) | `**(PREMIUM SAAS)**` | +| _Only_ GitLab Ultimate SaaS (no self-managed instances) | `**(ULTIMATE SAAS)**` | Topics that mention the `gitlab.rb` file are referring to self-managed instances of GitLab. To prevent confusion, include the relevant `TIER ONLY` @@ -1859,8 +1823,6 @@ Configuration procedures can require users to edit configuration files, reconfig GitLab, or restart GitLab. Use these styles to document these steps, replacing `PATH/TO` with the appropriate path: - - ````markdown **For Omnibus installations** @@ -1888,8 +1850,6 @@ GitLab, or restart GitLab. Use these styles to document these steps, replacing GitLab for the changes to take effect. ```` - - In this case: - Bold the installation method's name. @@ -1899,21 +1859,6 @@ In this case: - Use the [GitLab Restart](#gitlab-restart) section to explain any required restart or reconfigure of GitLab. -### Troubleshooting - -For troubleshooting sections, provide as much context as possible so -users can identify their problem and resolve it on their own. You -can facilitate this by making sure the troubleshooting content addresses: - -1. The problem the user needs to solve. -1. How the user can confirm they have the problem. -1. Steps the user can take towards resolution of the problem. - -If the contents of each category can be summarized in one line and a list of -steps aren't required, consider setting up a [table](#tables). Create headers of -_Problem_ \| _Cause_ \| _Solution_ (or _Workaround_ if the fix is temporary), or -_Error message_ \| _Solution_. - ## Feature flags Learn how to [document features deployed behind flags](../feature_flags.md). For diff --git a/doc/development/documentation/testing.md b/doc/development/documentation/testing.md index 561727648f0..f3d6e0a5c71 100644 --- a/doc/development/documentation/testing.md +++ b/doc/development/documentation/testing.md @@ -93,8 +93,8 @@ To execute documentation link tests locally: ### UI link tests -The `ui-docs-links lint` job uses `haml-lint` to test that all links to docs from -UI elements (`app/views` files, for example) are linking to valid docs and anchors. +The `ui-docs-links lint` job uses `haml-lint` to test that all documentation links from +UI elements (`app/views` files, for example) are linking to valid pages and anchors. To run the `ui-docs-links` test locally: @@ -156,12 +156,12 @@ markdownlint configuration is found in the following projects: - [`charts`](https://gitlab.com/gitlab-org/charts/gitlab/-/blob/master/.markdownlint.json) - [`gitlab-development-kit`](https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/master/.markdownlint.json) -This configuration is also used within build pipelines. +This configuration is also used in build pipelines. You can use markdownlint: - [On the command line](https://github.com/igorshubovych/markdownlint-cli#markdownlint-cli--). -- [Within a code editor](#configure-editors). +- [In a code editor](#configure-editors). - [In a `pre-push` hook](#configure-pre-push-hooks). ### Vale @@ -172,10 +172,10 @@ English language. Vale's configuration is stored in the directory of projects. Vale supports creating [custom tests](https://errata-ai.github.io/vale/styles/) that extend any of -several types of checks, which we store in the `.linting/vale/styles/gitlab` directory within the +several types of checks, which we store in the `.linting/vale/styles/gitlab` directory in the documentation directory of projects. -Vale configuration is found in the following projects: +You can find Vale configuration in the following projects: - [`gitlab`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/doc/.vale/gitlab) - [`gitlab-runner`](https://gitlab.com/gitlab-org/gitlab-runner/-/tree/master/docs/.vale/gitlab) @@ -183,13 +183,13 @@ Vale configuration is found in the following projects: - [`charts`](https://gitlab.com/gitlab-org/charts/gitlab/-/tree/master/doc/.vale/gitlab) - [`gitlab-development-kit`](https://gitlab.com/gitlab-org/gitlab-development-kit/-/tree/master/doc/.vale/gitlab) -This configuration is also used within build pipelines, where +This configuration is also used in build pipelines, where [error-level rules](#vale-result-types) are enforced. You can use Vale: - [On the command line](https://errata-ai.gitbook.io/vale/getting-started/usage). -- [Within a code editor](#configure-editors). +- [In a code editor](#configure-editors). - [In a Git hook](#configure-pre-push-hooks). Vale only reports errors in the Git hook (the same configuration as the CI/CD pipelines), and does not report suggestions or warnings. @@ -243,32 +243,40 @@ To match the versions of `markdownlint-cli` and `vale` used in the GitLab projec [versions used](https://gitlab.com/gitlab-org/gitlab-docs/-/blob/master/.gitlab-ci.yml#L447) when building the `image:docs-lint-markdown` Docker image containing these tools for CI/CD. -| Tool | Version | Command | Additional information | -|--------------------|----------|-------------------------------------------|------------------------| -| `markdownlint-cli` | Latest | `yarn global add markdownlint-cli` | n/a | -| `markdownlint-cli` | Specfic | `yarn global add markdownlint-cli@0.23.2` | The `@` indicates a specific version, and this example updates the tool to version `0.23.2`. | -| Vale | Latest | `brew update && brew upgrade vale` | This command is for macOS only. | -| Vale | Specific | n/a | Not possible using `brew`, but can be [directly downloaded](https://github.com/errata-ai/vale/releases). | +| Tool | Version | Command | Additional information | +|--------------------|-----------|-------------------------------------------|------------------------| +| `markdownlint-cli` | Latest | `yarn global add markdownlint-cli` | n/a | +| `markdownlint-cli` | Specific | `yarn global add markdownlint-cli@0.23.2` | The `@` indicates a specific version, and this example updates the tool to version `0.23.2`. | +| Vale | Latest | `brew update && brew upgrade vale` | This command is for macOS only. | +| Vale | Specific | n/a | Not possible using `brew`, but can be [directly downloaded](https://github.com/errata-ai/vale/releases). | ### Configure editors Using linters in your editor is more convenient than having to run the commands from the command line. -To configure markdownlint within your editor, install one of the following as appropriate: +To configure markdownlint in your editor, install one of the following as appropriate: -- [Sublime Text](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint) -- [Visual Studio Code](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) -- [Atom](https://atom.io/packages/linter-node-markdownlint) -- [Vim](https://github.com/dense-analysis/ale) +- Sublime Text [`SublimeLinter-contrib-markdownlint` package](https://packagecontrol.io/packages/SublimeLinter-contrib-markdownlint). +- Visual Studio Code [`DavidAnson.vscode-markdownlint` extension](https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint). +- Atom [`linter-node-markdownlint` package](https://atom.io/packages/linter-node-markdownlint). +- Vim [ALE plugin](https://github.com/dense-analysis/ale). -To configure Vale within your editor, install one of the following as appropriate: +To configure Vale in your editor, install one of the following as appropriate: -- The Sublime Text [`SublimeLinter-contrib-vale` plugin](https://packagecontrol.io/packages/SublimeLinter-contrib-vale). -- The Visual Studio Code [`errata-ai.vale-server` extension](https://marketplace.visualstudio.com/items?itemName=errata-ai.vale-server). - You don't need Vale Server to use the plugin. You can configure the plugin to +- Sublime Text [`SublimeLinter-contrib-vale` package](https://packagecontrol.io/packages/SublimeLinter-contrib-vale). +- Visual Studio Code [`errata-ai.vale-server` extension](https://marketplace.visualstudio.com/items?itemName=errata-ai.vale-server). + You can configure the plugin to [display only a subset of alerts](#show-subset-of-vale-alerts). -- [Vim](https://github.com/dense-analysis/ale). + + In the extension's settings: + + - Select the **Use CLI** checkbox. + - In the **Config** setting, enter an absolute path to [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/blob/master/.vale.ini) in one of the cloned GitLab repositories on your computer. + - In the **Path** setting, enter the absolute path to the Vale binary. In most + cases, `vale` should work. To find the location, run `which vale` in a terminal. + +- Vim [ALE plugin](https://github.com/dense-analysis/ale). We don't use [Vale Server](https://errata-ai.github.io/vale/#using-vale-with-a-text-editor-or-another-third-party-application). @@ -286,7 +294,7 @@ Configuration for `lefthook` is available in the [`lefthook.yml`](https://gitlab file for the [`gitlab`](https://gitlab.com/gitlab-org/gitlab) project. To set up `lefthook` for documentation linting, see -[Pre-push static analysis](../contributing/style_guides.md#pre-push-static-analysis). +[Pre-push static analysis](../contributing/style_guides.md#pre-push-static-analysis-with-lefthook). ### Show subset of Vale alerts diff --git a/doc/development/documentation/workflow.md b/doc/development/documentation/workflow.md index e809ca84707..8e2028532e4 100644 --- a/doc/development/documentation/workflow.md +++ b/doc/development/documentation/workflow.md @@ -65,13 +65,15 @@ To update GitLab documentation: NOTE: Work in a fork if you do not have Developer access to the GitLab project. -Request help from the Technical Writing team if you: +### Ask for help + +Ask for help from the Technical Writing team 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. -To request help: +To identify someone who can help you: 1. Locate the Technical Writer for the relevant [DevOps stage group](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments). diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md index 5be601187ca..81014b7624c 100644 --- a/doc/development/ee_features.md +++ b/doc/development/ee_features.md @@ -92,12 +92,36 @@ class User < ActiveRecord::Base # ... lots of code here ... end -User.prepend_if_ee('EE::User') +User.prepend_ee_mod ``` Do not use methods such as `prepend`, `extend`, and `include`. Instead, use -`prepend_if_ee`, `extend_if_ee`, or `include_if_ee`. These methods take a -_String_ containing the full module name as the argument, not the module itself. +`prepend_ee_mod`, `extend_ee_mod`, or `include_ee_mod`. These methods will try to +find the relevant EE module by the name of the receiver module, for example; + +```ruby +module Vulnerabilities + class Finding + #... + end +end + +Vulnerabilities::Finding.prepend_ee_mod +``` + +will prepend the module named `::EE::Vulnerabilities::Finding`. + +If the extending module does not follow this naming convention, you can also provide the module name +by using `prepend_if_ee`, `extend_if_ee`, or `include_if_ee`. These methods take a +_String_ containing the full module name as the argument, not the module itself, like so; + +```ruby +class User + #... +end + +User.prepend_if_ee('::EE::UserExtension') +``` Since the module would require an `EE` namespace, the file should also be put in an `ee/` sub-directory. For example, we want to extend the user model @@ -142,9 +166,9 @@ 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#override) and use `override` to - guard the "overrider" method to ensure that if the method gets renamed in + guard the `overrider` method to ensure that if the method gets renamed in CE, the EE override isn't silently forgotten. -- when the "overrider" would add a line in the middle of the CE +- when the `overrider` would add a line in the middle of the CE implementation, you should refactor the CE method and split it in smaller methods. Or create a "hook" method that is empty in CE, and with the EE-specific implementation in EE. @@ -947,7 +971,7 @@ information on managing page-specific JavaScript within EE. #### Child Component only used in EE -To separate Vue template differences we should [async import the components](https://vuejs.org/v2/guide/components-dynamic-async.html#Async-Components). +To separate Vue template differences we should [import the components asynchronously](https://vuejs.org/v2/guide/components-dynamic-async.html#Async-Components). Doing this allows for us to load the correct component in EE while in CE we can load a empty component that renders nothing. This code **should** @@ -1044,7 +1068,7 @@ export default { **For EE components that need different results for the same computed values, we can pass in props to the CE wrapper as seen in the example.** - **EE Child components** - - Since we are using the async loading to check which component to load, we'd still use the component's name, check [this example](#child-component-only-used-in-ee). + - Since we are using the asynchronous loading to check which component to load, we'd still use the component's name, check [this example](#child-component-only-used-in-ee). - **EE extra HTML** - For the templates that have extra HTML in EE we should move it into a new component and use the `ee_else_ce` dynamic import diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 8bf8a5fccb8..3392bd1fbf6 100644 --- a/doc/development/elasticsearch.md +++ b/doc/development/elasticsearch.md @@ -4,7 +4,7 @@ group: Global Search info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- -# Elasticsearch knowledge **(STARTER ONLY)** +# Elasticsearch knowledge **(PREMIUM SELF)** This area is to maintain a compendium of useful information when working with Elasticsearch. @@ -13,9 +13,9 @@ the [Elasticsearch integration documentation](../integration/elasticsearch.md#en ## Deep Dive -In June 2019, Mario de la Ossa hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) on the GitLab [Elasticsearch integration](../integration/elasticsearch.md) to share his domain specific knowledge with anyone who may work in this part of the codebase in the future. You can find the [recording on YouTube](https://www.youtube.com/watch?v=vrvl-tN2EaA), and the slides on [Google Slides](https://docs.google.com/presentation/d/1H-pCzI_LNrgrL5pJAIQgvLX8Ji0-jIKOg1QeJQzChug/edit) and in [PDF](https://gitlab.com/gitlab-org/create-stage/uploads/c5aa32b6b07476fa8b597004899ec538/Elasticsearch_Deep_Dive.pdf). Everything covered in this deep dive was accurate as of GitLab 12.0, and while specific details may have changed since then, it should still serve as a good introduction. +In June 2019, Mario de la Ossa hosted a Deep Dive (GitLab team members only: `https://gitlab.com/gitlab-org/create-stage/issues/1`) on the GitLab [Elasticsearch integration](../integration/elasticsearch.md) to share his domain specific knowledge with anyone who may work in this part of the codebase in the future. You can find the [recording on YouTube](https://www.youtube.com/watch?v=vrvl-tN2EaA), and the slides on [Google Slides](https://docs.google.com/presentation/d/1H-pCzI_LNrgrL5pJAIQgvLX8Ji0-jIKOg1QeJQzChug/edit) and in [PDF](https://gitlab.com/gitlab-org/create-stage/uploads/c5aa32b6b07476fa8b597004899ec538/Elasticsearch_Deep_Dive.pdf). Everything covered in this deep dive was accurate as of GitLab 12.0, and while specific details may have changed since then, it should still serve as a good introduction. -In August 2020, a second Deep Dive was hosted, focusing on [GitLab-specific architecture for multi-indices support](#zero-downtime-reindexing-with-multiple-indices). The [recording on YouTube](https://www.youtube.com/watch?v=0WdPR9oB2fg) and the [slides](https://lulalala.gitlab.io/gitlab-elasticsearch-deepdive/) are available. Everything covered in this deep dive was accurate as of GitLab 13.3. +In August 2020, a second Deep Dive was hosted, focusing on [GitLab-specific architecture for multi-indices support](#zero-downtime-reindexing-with-multiple-indices). The [recording on YouTube](https://www.youtube.com/watch?v=0WdPR9oB2fg) and the [slides](https://lulalala.gitlab.io/gitlab-elasticsearch-deepdive/) are available. Everything covered in this deep dive was accurate as of GitLab 13.3. ## Supported Versions @@ -69,7 +69,7 @@ The `whitespace` tokenizer was selected in order to have more control over how t Please see the `code` filter for an explanation on how tokens are split. NOTE: -Currently the [Elasticsearch code_analyzer doesn't account for all code cases](../integration/elasticsearch.md#known-issues). +The [Elasticsearch code_analyzer doesn't account for all code cases](../integration/elasticsearch.md#elasticsearch-code_analyzer-doesnt-account-for-all-code-cases). #### `code_search_analyzer` @@ -210,7 +210,7 @@ class MigrationName < Elastic::Migration end ``` -Applied migrations are stored in `gitlab-#{RAILS_ENV}-migrations` index. All unexecuted migrations +Applied migrations are stored in `gitlab-#{RAILS_ENV}-migrations` index. All migrations not executed are applied by the [`Elastic::MigrationWorker`](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/workers/elastic/migration_worker.rb) cron worker sequentially. @@ -337,7 +337,7 @@ cluster.routing.allocation.disk.watermark.low: 15gb cluster.routing.allocation.disk.watermark.high: 10gb ``` -Restart Elasticsearch, and the `read_only_allow_delete` will clear on it's own. +Restart Elasticsearch, and the `read_only_allow_delete` will clear on its 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.7/disk-allocator.html)_ @@ -351,7 +351,7 @@ simply reindex everything from scratch. If your Elasticsearch index is incredibly large it may be too time consuming or cause too much downtime to reindex from scratch. There aren't any built in -mechanisms for automatically finding discrepencies and resyncing an +mechanisms for automatically finding discrepancies and resyncing an Elasticsearch index if it gets out of sync but one tool that may be useful is looking at the logs for all the updates that occurred in a time range you believe may have been missed. This information is very low level and only diff --git a/doc/development/experiment_guide/index.md b/doc/development/experiment_guide/index.md index a1899ab5f18..21c61324dc1 100644 --- a/doc/development/experiment_guide/index.md +++ b/doc/development/experiment_guide/index.md @@ -12,7 +12,7 @@ Experiments are run as an A/B test and are behind a feature flag to turn the tes ## Experiment tracking issue -Each experiment should have an [Experiment tracking](https://gitlab.com/groups/gitlab-org/-/issues?scope=all&utf8=%E2%9C%93&state=opened&label_name[]=growth%20experiment&search=%22Experiment+tracking%22) issue to track the experiment from roll-out through to cleanup/removal. Immediately after an experiment is deployed, the due date of the issue should be set (this depends on the experiment but can be up to a few weeks in the future). +Each experiment should have an [Experiment tracking](https://gitlab.com/groups/gitlab-org/-/issues?scope=all&utf8=%E2%9C%93&state=opened&label_name[]=growth%20experiment&search=%22Experiment+tracking%22) issue to track the experiment from roll-out through to cleanup/removal. The tracking issue is similar to a feature flag rollout issue, and is also used to track the status of an experiment. Immediately after an experiment is deployed, the due date of the issue should 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 becomes the new default. @@ -36,7 +36,17 @@ and link to the issue that resolves the experiment. If the experiment is successful and becomes part of the product, any follow up issues should be addressed. -## How to create an A/B test +## Experiments using `gitlab-experiment` + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/300383) in GitLab 13.7. +> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default. +> - It's enabled on GitLab.com. +> - It is not yet intended for use in GitLab self-managed instances. + +[GitLab Experiment](https://gitlab.com/gitlab-org/gitlab-experiment/) is a gem included +in GitLab that can be used for running experiments. + +## How to create an A/B test using `experimentation.rb` ### Implement the experiment @@ -315,7 +325,7 @@ Note that the use of this method requires that we have first [recorded the user ### Enable the experiment -After all merge requests have been merged, use [`chatops`](../../ci/chatops/README.md) in the +After all merge requests have been merged, use [`chatops`](../../ci/chatops/index.md) in the [appropriate channel](../feature_flags/controls.md#communicate-the-change) to start the experiment for 10% of the users. The feature flag should have the name of the experiment with the `_experiment_percentage` suffix appended. For visibility, please also share any commands run against production in the `#s_growth` channel: @@ -358,7 +368,7 @@ Use a comma to list more than one experiment to be forced: document.cookie = "force_experiment=,; path=/"; ``` -Clear the experiments by unsetting the `force_experiment` cookie: +To clear the experiments, unset the `force_experiment` cookie: ```javascript document.cookie = "force_experiment=; path=/"; diff --git a/doc/development/export_csv.md b/doc/development/export_csv.md index 99b6062736d..0bf12149779 100644 --- a/doc/development/export_csv.md +++ b/doc/development/export_csv.md @@ -14,7 +14,7 @@ This document lists the different implementations of CSV export in GitLab codeba | Downloading | - Query and write data in batches to a temporary file.
- Loads the file into memory.
- Sends the file to the client. | - Report available immediately. | - Large amount of data might cause request timeout.
- Memory intensive.
- Request expires when user navigates to a different page. | [Export Chain of Custody Report](../user/compliance/compliance_dashboard/#chain-of-custody-report) | | As email attachment | - Asynchronously process the query with background job.
- Email uses the export as an attachment. | - Asynchronous processing. | - Requires users use a different app (email) to download the CSV.
- Email providers may limit attachment size. | - [Export Issues](../user/project/issues/csv_export.md)
- [Export Merge Requests](../user/project/merge_requests/csv_export.md) | | As downloadable link in email (*) | - Asynchronously process the query with background job.
- Email uses an export link. | - Asynchronous processing.
- Bypasses email provider attachment size limit. | - Requires users use a different app (email).
- Requires additional storage and cleanup. | [Export User Permissions](https://gitlab.com/gitlab-org/gitlab/-/issues/1772) | -| Polling (non-persistent state) | - Asynchronously processes the query with the background job.
- Frontend(FE) polls every few seconds to check if CSV file is ready. | - Asynchronous processing.
- Automatically downloads to local machine on completion.
- In-app solution. | - Non-persistable request - request expires when user navigates to a different page.
- API is processed for each polling request. | [Export Vulnerabilities](../user/application_security/security_dashboard/#export-vulnerabilities) | +| Polling (non-persistent state) | - Asynchronously processes the query with the background job.
- Frontend(FE) polls every few seconds to check if CSV file is ready. | - Asynchronous processing.
- Automatically downloads to local machine on completion.
- In-app solution. | - Non-persistable request - request expires when user navigates to a different page.
- API is processed for each polling request. | [Export Vulnerabilities](../user/application_security/vulnerability_report/#export-vulnerabilities) | | Polling (persistent state) (*) | - Asynchronously processes the query with background job.
- Backend (BE) maintains the export state
- FE polls every few seconds to check status.
- FE shows 'Download link' when export is ready.
- User can download or regenerate a new report. | - Asynchronous processing.
- No database calls made during the polling requests (HTTP 304 status is returned until export status changes).
- Does not require user to stay on page until export is complete.
- In-app solution.
- Can be expanded into a generic CSV feature (such as dashboard / CSV API). | - Requires to maintain export states in DB.
- Does not automatically download the CSV export to local machine, requires users to click 'Download' button. | [Export Merge Commits Report](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43055) | NOTE: diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 92730e8139f..5ad1a701fac 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -9,11 +9,9 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Resources [Chrome Accessibility Developer Tools](https://github.com/GoogleChrome/accessibility-developer-tools) -are useful for testing for potential accessibility problems in GitLab. +assist with testing for potential accessibility problems in GitLab. -The [axe](https://www.deque.com/axe/) browser extension (available for [Firefox](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/) and [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd)) is -also a handy tool for running audits and getting feedback on markup, CSS and even potentially problematic color usages. +The [axe](https://www.deque.com/axe/) browser extension (available for [Firefox](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/) and [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd)) provides running audits and feedback on markup, CSS, and even potentially problematic color usages. Accessibility best-practices and more in-depth information are available on -[the Audit Rules page](https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules) for the Chrome Accessibility Developer Tools. The [Awesome Accessibility](https://github.com/brunopulis/awesome-a11y) list is also a -useful compilation of accessibility-related material. +[the Audit Rules page](https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules) for the Chrome Accessibility Developer Tools. The [Awesome Accessibility](https://github.com/brunopulis/awesome-a11y) list is a compilation of accessibility-related material. diff --git a/doc/development/fe_guide/architecture.md b/doc/development/fe_guide/architecture.md index 964837dc5f7..c51f99ca9d2 100644 --- a/doc/development/fe_guide/architecture.md +++ b/doc/development/fe_guide/architecture.md @@ -6,9 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Architecture -When you are developing a new feature that requires architectural design, or if -you are changing the fundamental design of an existing feature, make sure it is -discussed with one of the Frontend Architecture Experts. +When developing a feature that requires architectural design, or changing the fundamental design of an existing feature, discuss it with a Frontend Architecture Expert. A Frontend Architect is an expert who makes high-level Frontend design decisions and decides on technical standards, including coding standards and frameworks. diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md index cf5a4970c04..2d699b305ce 100644 --- a/doc/development/fe_guide/axios.md +++ b/doc/development/fe_guide/axios.md @@ -44,7 +44,7 @@ Advantages over [`spyOn()`](https://jasmine.github.io/api/edge/global.html#spyOn - no need to create response objects - does not allow call through (which we want to avoid) -- simple API to test error cases +- clear API to test error cases - provides `replyOnce()` to allow for different responses We have also decided against using [Axios interceptors](https://github.com/axios/axios#interceptors) because they are not suitable for mocking. diff --git a/doc/development/fe_guide/components.md b/doc/development/fe_guide/components.md deleted file mode 100644 index 6b6274a6480..00000000000 --- a/doc/development/fe_guide/components.md +++ /dev/null @@ -1,3 +0,0 @@ ---- -redirect_to: 'https://design.gitlab.com/components/status/' ---- diff --git a/doc/development/fe_guide/dark_mode.md b/doc/development/fe_guide/dark_mode.md new file mode 100644 index 00000000000..dd7ffd1ee6c --- /dev/null +++ b/doc/development/fe_guide/dark_mode.md @@ -0,0 +1,77 @@ +--- +type: reference, dev +stage: none +group: Development +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +This page is about developing dark mode for GitLab. We also have documentation on how +[to enable dark mode](../../user/profile/preferences.md#dark-mode). + +# How dark mode works + +Short version: Reverse the color palette and override a few Bootstrap variables. + +Note the following: + +- The dark mode palette is defined in `app/assets/stylesheets/themes/_dark.scss`. + This is loaded _before_ application.scss to generate `application_dark.css` + - We define two types of variables in `_dark.scss`: + - SCSS variables are used in framework, components, and utitlity classes. + - CSS variables are used for any colors within the `app/assets/stylesheets/page_bundles` directory. +- `app/views/layouts/_head.html.haml` then loads application or application_dark based on the user's theme preference. + +As we do not want to generate separate `_dark.css` variants of every page_bundle file, +we use CSS variables with SCSS variables as fallbacks. This is because when we generate the `page_bundles` +CSS, we get the variable values from `_variables.scss`, so any SCSS variables have light mode values. + +As the CSS variables defined in `_dark.scss` are available in the browser, they have the +correct colors for dark mode. + +```scss +color: var(--gray-500, $gray-500); +``` + +## Utility classes + +We generate a separate `utilities_dark.css` file for utility classes containing the inverted values. So a class +such as `gl-text-white` specifies a text color of `#333` in dark mode. This means you do not have to +add multiple classes every time you want to add a color. + +Currently, we cannot set up a utility class only in dark mode. We hope to address that +[issue](https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1141) soon. + +## Using different values in light and dark mode + +In most cases, we can use the same values for light and dark mode. If that is not possible, you +can add an override using the `.gl-dark` class that dark mode adds to `body`: + +```scss +color: $gray-700; +.gl-dark & { + color: var(--gray-500); +} +``` + +NOTE: +Avoid using a different value for the SCSS fallback + +```scss +// avoid where possible +// --gray-500 (#999) in dark mode +// $gray-700 (#525252) in light mode +color: var(--gray-500, $gray-700); +``` + +We [plan to add](https://gitlab.com/gitlab-org/gitlab/-/issues/301147) the CSS variables to light mode. When that happens, different values for the SCSS fallback will no longer work. + +## When to use SCSS variables + +There are a few things we do in SCSS that we cannot (easily) do with CSS, such as the following +functions: + +- `lighten` +- `darken` +- `color-yiq` (color contrast) + +If those are needed then SCSS variables should be used. diff --git a/doc/development/fe_guide/dependencies.md b/doc/development/fe_guide/dependencies.md index b036819cde1..8fe03544f85 100644 --- a/doc/development/fe_guide/dependencies.md +++ b/doc/development/fe_guide/dependencies.md @@ -6,28 +6,75 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Frontend dependencies -## Package manager +We use [yarn@1](https://classic.yarnpkg.com/lang/en/) to manage frontend dependencies. -We use [Yarn](https://yarnpkg.com/) to manage frontend dependencies. There are a few exceptions, stored in `vendor/assets/`. +There are a few exceptions in the GitLab repository, stored in `vendor/assets/`. -## Updating dependencies +## What are production and development dependencies? + +These dependencies are defined in two groups within `package.json`, `dependencies` and `devDependencies`. +For our purposes, we consider anything that is required to compile our production assets a "production" dependency. +That is, anything required to run the `webpack` script with `NODE_ENV=production`. +Tools like `eslint`, `jest`, and various plugins and tools used in development are considered `devDependencies`. +This distinction is used by omnibus to determine which dependencies it requires when building GitLab. -### Renovate GitLab Bot +Exceptions are made for some tools that we require in the +`compile-production-assets` CI job such as `webpack-bundle-analyzer` to analyze our +production assets post-compile. + +## Updating dependencies We use the [Renovate GitLab Bot](https://gitlab.com/gitlab-org/frontend/renovate-gitlab-bot) to -automatically create merge requests for updating dependencies of several projects. You can find the -up-to-date list of projects managed by the renovate bot in the project’s README. Some key dependencies -updated using renovate are: +automatically create merge requests for updating dependencies of several projects. +You can find the up-to-date list of projects managed by the renovate bot in the project’s README. + +Some key dependencies updated using renovate are: - [`@gitlab/ui`](https://gitlab.com/gitlab-org/gitlab-ui) - [`@gitlab/svgs`](https://gitlab.com/gitlab-org/gitlab-svgs) - [`@gitlab/eslint-plugin`](https://gitlab.com/gitlab-org/frontend/eslint-plugin) +- And any other package in the `@gitlab/` scope + +We have the goal of updating [_all_ dependencies with renovate](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/21). + +Updating dependencies automatically has several benefits, have a look at this [example MR](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53613). + +- MRs will be created automatically when new versions are released +- MRs can easily be rebased and updated with just checking a checkbox in the MR description +- MRs contain changelog summaries and links to compare the different package versions +- MRs can be assigned to people directly responsible for the dependencies + +### Community contributions updating dependencies + +It is okay to reject Community Contributions that solely bump dependencies. +Simple dependency updates are better done automatically for the reasons provided above. +If a community contribution needs to be rebased, runs into conflicts, or goes stale, the effort required +to instruct the contributor to correct it often outweighs the benefits. + +If a dependency update is accompanied with significant migration efforts, due to major version updates, +a community contribution is acceptable. + +Here is a message you can use to explain to community contributors as to why we reject simple updates: + +```markdown +Hello CONTRIBUTOR! + +Thank you very much for this contribution. It seems like you are doing a "simple" dependency update. + +If a dependency update is as simple as increasing the version number, we'd like a Bot to do this to save you and ourselves some time. + +This has certain benefits as outlined in our Frontend development guidelines. + +You might find that we do not currently update DEPENDENCY automatically, but we are planning to do so in [the near future](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/21). + +Thank you for understanding, I will close this Merge Request. +/close +``` ### Blocked dependencies -We discourage installing some dependencies in [GitLab repository](https://gitlab.com/gitlab-org/gitlab) -because they can create conflicts in the dependency tree. Blocked dependencies are declared in the -`blockDependencies` property of the GitLab [`package.json` file](https://gitlab.com/gitlab-org/gitlab/-/blob/master/package.json). +We discourage installing some dependencies in [GitLab repository](https://gitlab.com/gitlab-org/gitlab) because they can create conflicts in the dependency tree. +Blocked dependencies are declared in the `blockDependencies` property of the GitLab [`package.json`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/package.json). ## Dependency notes diff --git a/doc/development/fe_guide/design_anti_patterns.md b/doc/development/fe_guide/design_anti_patterns.md new file mode 100644 index 00000000000..d230e413879 --- /dev/null +++ b/doc/development/fe_guide/design_anti_patterns.md @@ -0,0 +1,219 @@ +--- +stage: none +group: unassigned +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Design Anti-patterns + +Anti-patterns may seem like good approaches at first, but it has been shown that they bring more ills than benefits. These should +generally be avoided. + +Throughout the GitLab codebase, there may be historic uses of these anti-patterns. Please [use discretion](https://about.gitlab.com/handbook/engineering/#balance-refactoring-and-velocity) +when figuring out whether or not to refactor, when touching code that uses one of these legacy patterns. + +**Please note:** For new features, anti-patterns are not necessarily prohibited, but it is **strongly suggested** to find another approach. + +## Shared Global Object (Anti-pattern) + +A shared global object is an instance of something that can be accessed from anywhere and therefore has no clear owner. + +Here's an example of this pattern applied to a Vuex Store: + +```javascript +const createStore = () => new Vuex.Store({ + actions, + state, + mutations +}); + +// Notice that we are forcing all references to this module to use the same single instance of the store. +// We are also creating the store at import-time and there is nothing which can automatically dispose of it. +// +// As an alternative, we should simply export the `createStore` and let the client manage the +// lifecycle and instance of the store. +export default createStore(); +``` + +### What problems do Shared Global Objects cause? + +Shared Global Objects are convenient because they can be accessed from anywhere. However, +the convenience does not always outweigh their heavy cost: + +- **No ownership.** There is no clear owner to these objects and therefore they assume a non-deterministic + and permanent lifecycle. This can be especially problematic for tests. +- **No access control.** When Shared Global Objects manage some state, this can create some very buggy and difficult + coupling situations because there is no access control to this object. +- **Possible circular references.** Shared Global Objects can also create some circular referencing situations since submodules + of the Shared Global Object can reference modules that reference itself (see + [this MR for an example](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33366)). + +Here are some historic examples where this pattern was identified to be problematic: + +- [Reference to global Vuex store in IDE](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36401) +- [Docs update to discourage singleton Vuex store](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36952) + +### When could the Shared Global Object pattern be actually appropriate? + +Shared Global Object's solve the problem of making something globally accessible. This pattern +could be appropriate: + +- When a responsibility is truly global and should be referenced across the application + (e.g., an application-wide Event Bus). + +Even in these scenarios, please consider avoiding the Shared Global Object pattern because the +side-effects can be notoriously difficult to reason with. + +### References + +To read more on this topic, check out the following references: + +- [GlobalVariablesAreBad from C2 wiki](https://wiki.c2.com/?GlobalVariablesAreBad) + +## Singleton (Anti-pattern) + +The classic [Singleton pattern](https://en.wikipedia.org/wiki/Singleton_pattern) is an approach to ensure that only one +instance of a thing exists. + +Here's an example of this pattern: + +```javascript +class MyThing { + constructor() { + // ... + } + + // ... +} + +MyThing.instance = null; + +export const getThingInstance = () => { + if (MyThing.instance) { + return MyThing.instance; + } + + const instance = new MyThing(); + MyThing.instance = instance; + return instance; +}; +``` + +### What problems do Singletons cause? + +It is a big assumption that only one instance of a thing should exist. More often than not, +a Singleton is misused and causes very tight coupling amongst itself and the modules that reference it. + +Here are some historic examples where this pattern was identified to be problematic: + +- [Test issues caused by singleton class in IDE](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30398#note_331174190) +- [Implicit Singleton created by module's shared variables](https://gitlab.com/gitlab-org/gitlab-vscode-extension/-/merge_requests/97#note_417515776) +- [Complexity caused by Singletons](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29461#note_324585814) + +Here are some ills that Singletons often produce: + +1. **Non-deterministic tests.** Singletons encourage non-deterministic tests because the single instance is shared across + individual tests, often causing the state of one test to bleed into another. +1. **High coupling.** Under the hood, clients of a singleton class all share a single specific + instance of an object, which means this pattern inherits all the [problems of Shared Global Object](#what-problems-do-shared-global-objects-cause) + such as no clear ownership and no access control. These leads to high coupling situations that can + be buggy and difficult to untangle. +1. **Infectious.** Singletons are infectious, especially when they manage state. Consider the component + [RepoEditor](https://gitlab.com/gitlab-org/gitlab/blob/27ad6cb7b76430fbcbaf850df68c338d6719ed2b/app%2Fassets%2Fjavascripts%2Fide%2Fcomponents%2Frepo_editor.vue#L0-1) + used in the Web IDE. This component interfaces with a Singleton [Editor](https://gitlab.com/gitlab-org/gitlab/blob/862ad57c44ec758ef3942ac2e7a2bd40a37a9c59/app%2Fassets%2Fjavascripts%2Fide%2Flib%2Feditor.js#L21) + which manages some state for working with Monaco. Because of the Singleton nature of the Editor class, + the component `RepoEditor` is now forced to be a Singleton as well. Multiple instances of this component + would cause production issues because no one truly owns the instance of `Editor`. + +### Why is the Singleton pattern popular in other languages like Java? + +This is because of the limitations of languages like Java where everything has to be wrapped +in a class. In JavaScript we have things like object and function literals where we can solve +many problems with a module that simply exports utility functions. + +### When could the Singleton pattern be actually appropriate?** + +Singletons solve the problem of enforcing there to be only 1 instance of a thing. It's possible +that a Singleton could be appropriate in the following rare cases: + +- We need to manage some resource that **MUST** have just 1 instance (i.e. some hardware restriction). +- There is a real [cross-cutting concern](https://en.wikipedia.org/wiki/Cross-cutting_concern) (e.g., logging) and a Singleton provides the simplest API. + +Even in these scenarios, please consider avoiding the Singleton pattern. + +### What alternatives are there to the Singleton pattern? + +#### Utility Functions + +When no state needs to be managed, we can simply export utility functions from a module without +messing with any class instantiation. + +```javascript +// bad - Singleton +export class ThingUtils { + static create() { + if(this.instance) { + return this.instance; + } + + this.instance = new ThingUtils(); + return this.instance; + } + + bar() { /* ... */ } + + fuzzify(id) { /* ... */ } +} + +// good - Utility functions +export const bar = () => { /* ... */ }; + +export const fuzzify = (id) => { /* ... */ }; +``` + +#### Dependency Injection + +[Dependency Injection](https://en.wikipedia.org/wiki/Dependency_injection) is an approach which breaks +coupling by declaring a module's dependencies to be injected from outside the module (e.g., through constructor parameters, a bona-fide Dependency Injection framework, and even Vue's `provide/inject`). + +```javascript +// bad - Vue component coupled to Singleton +export default { + created() { + this.mediator = MyFooMediator.getInstance(); + }, +}; + +// good - Vue component declares dependency +export default { + inject: ['mediator'] +}; +``` + +```javascript +// bad - We're not sure where the singleton is in it's lifecycle so we init it here. +export class Foo { + constructor() { + Bar.getInstance().init(); + } + + stuff() { + return Bar.getInstance().doStuff(); + } +} + +// good - Lets receive this dependency as a constructor argument. +// It's also not our responsibility to manage the lifecycle. +export class Foo { + constructor(bar) { + this.bar = bar; + } + + stuff() { + return this.bar.doStuff(); + } +} +``` + +In this example, the lifecycle and implementation details of `mediator` are all managed +**outside** the component (most likely the page entrypoint). diff --git a/doc/development/fe_guide/design_patterns.md b/doc/development/fe_guide/design_patterns.md index 784612682f8..c769d0767e7 100644 --- a/doc/development/fe_guide/design_patterns.md +++ b/doc/development/fe_guide/design_patterns.md @@ -6,79 +6,11 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Design Patterns -## Singletons +The following design patterns are suggested approaches for solving common problems. Use discretion when evaluating +if a certain pattern makes sense in your situation. Just because it is a pattern, doesn't mean it is a good one for your problem. -When exactly one object is needed for a given task, prefer to define it as a -`class` rather than as an object literal. Prefer also to explicitly restrict -instantiation, unless flexibility is important (e.g. for testing). +**Please note:** When adding a design pattern to this document, be sure to clearly state the **problem it solves**. -```javascript -// bad +## TBD -const MyThing = { - prop1: 'hello', - method1: () => {} -}; - -export default MyThing; - -// good - -class MyThing { - constructor() { - this.prop1 = 'hello'; - } - method1() {} -} - -export default new MyThing(); - -// best - -export default class MyThing { - constructor() { - if (!MyThing.prototype.singleton) { - this.init(); - MyThing.prototype.singleton = this; - } - return MyThing.prototype.singleton; - } - - init() { - this.prop1 = 'hello'; - } - - method1() {} -} - -``` - -## Manipulating the DOM in a JS Class - -When writing a class that needs to manipulate the DOM guarantee a container option is provided. -This is useful when we need that class to be instantiated more than once in the same page. - -Bad: - -```javascript -class Foo { - constructor() { - document.querySelector('.bar'); - } -} -new Foo(); -``` - -Good: - -```javascript -class Foo { - constructor(opts) { - document.querySelector(`${opts.container} .bar`); - } -} - -new Foo({ container: '.my-element' }); -``` - -You can find an example of the above in this [class](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js); +Stay tuned! diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index d122459f51c..b85ed4da442 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -12,9 +12,9 @@ You can find more about the organization of the frontend team in the [handbook]( The idea is to remind us about specific topics during the time we build a new feature or start something. This is a common practice in other industries (like pilots) that also use standardized checklists to reduce problems early on. -Copy the content over to your issue or merge request and if something doesn't apply simply remove it from your current list. +Copy the content over to your issue or merge request and if something doesn't apply, remove it from your current list. -This checklist is intended to help us during development of bigger features/refactorings, it's not a "use it always and every point always matches" list. +This checklist is intended to help us during development of bigger features/refactorings. It is not a "use it always and every point always matches" list. Please use your best judgment when to use it and please contribute new points through merge requests if something comes to your mind. @@ -77,7 +77,7 @@ With the purpose of being [respectful of others' time](https://about.gitlab.com/ - includes tests - includes a changelog entry (when necessary) - Before assigning to a maintainer, assign to a reviewer. -- If you assigned a merge request or pinged someone directly, be patient because we work in different timezones and asynchronously. Unless the merge request is urgent (like fixing a broken master), please don't DM or reassign the merge request before waiting for a 24-hour window. +- If you assigned a merge request or pinged someone directly, be patient because we work in different timezones and asynchronously. Unless the merge request is urgent (like fixing a broken default branch), please don't DM or reassign the merge request before waiting for a 24-hour window. - If you have a question regarding your merge request/issue, make it on the merge request/issue. When we DM each other, we no longer have a SSOT and [no one else is able to contribute](https://about.gitlab.com/handbook/values/#public-by-default). - When you have a big **Draft** merge request with many changes, you're advised to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before the **Draft** ones. - Make sure to remove the `Draft:` title before the last round of review. diff --git a/doc/development/fe_guide/dropdowns.md b/doc/development/fe_guide/dropdowns.md deleted file mode 100644 index bd2dae13c5b..00000000000 --- a/doc/development/fe_guide/dropdowns.md +++ /dev/null @@ -1,3 +0,0 @@ ---- -redirect_to: 'https://design.gitlab.com/components/dropdown/' ---- diff --git a/doc/development/fe_guide/editor_lite.md b/doc/development/fe_guide/editor_lite.md index 47ef85d8737..f783a97fbd3 100644 --- a/doc/development/fe_guide/editor_lite.md +++ b/doc/development/fe_guide/editor_lite.md @@ -8,13 +8,13 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Background -**Editor Lite** is a technological product driving [Web Editor](../../user/project/repository/web_editor.md), [Snippets](../../user/snippets.md), [CI Linter](../../ci/lint.md), etc. Editor Lite is the driving technology for any single-file editing experience across the product. +**Editor Lite** is a technological product driving features like [Web Editor](../../user/project/repository/web_editor.md), [Snippets](../../user/snippets.md), and [CI Linter](../../ci/lint.md). Editor Lite is the driving technology for any single-file editing experience across the product. Editor Lite is a thin wrapper around [the Monaco editor](https://microsoft.github.io/monaco-editor/index.html) that provides the necessary helpers and abstractions and extends Monaco using extensions. ## How to use Editor Lite -Editor Lite is framework-agnostic and can be used in any application, whether it's Rails or Vue. For the convenience of integration, we have [the dedicated `` Vue component](#vue-component), but in general, the integration of Editor Lite is pretty straightforward: +Editor Lite is framework-agnostic and can be used in any application, whether it's Rails or Vue. For the convenience of integration, we have the dedicated `` Vue component, but in general, the integration of Editor Lite is pretty straightforward: 1. Import Editor Lite: @@ -65,7 +65,7 @@ The editor follows the same public API as [provided by Monaco editor](https://mi 1. Editor's loading state. -Editor Lite comes with the loading state built-in, making spinners and loaders rarely needed in HTML. To benefit the built-in loading state, set the `data-editor-loading` property on the HTML element that is supposed to contain the editor. Editor Lite will show the loader automatically while it's bootstrapping. +Editor Lite comes with the loading state built-in, making spinners and loaders rarely needed in HTML. To benefit the built-in loading state, set the `data-editor-loading` property on the HTML element that is supposed to contain the editor. Editor Lite shows the loader automatically while it's bootstrapping. ![Editor Lite: loading state](img/editor_lite_loading.png) 1. Update syntax highlighting if the filename changes. @@ -89,7 +89,7 @@ form.addEventListener('submit', () => { 1. Performance -Even though Editor Lite itself is extremely slim, it still depends on Monaco editor. Monaco is not an easily tree-shakeable module. Hence, every time you add Editor Lite to a view, the JavaScript bundle's size significantly increases, affecting your view's loading performance. To avoid that, it is recommended to import the editor on demand on those views where it is not 100% certain that the editor will be used. Or if the editor is a secondary element of the view. Loading Editor Lite on demand is no different from loading any other module: +Even though Editor Lite itself is extremely slim, it still depends on Monaco editor. Monaco is not an easily tree-shakeable module. Hence, every time you add Editor Lite to a view, the JavaScript bundle's size significantly increases, affecting your view's loading performance. It is recommended to import the editor on demand on those views where it is not 100% certain that the editor is needed. Or if the editor is a secondary element of the view. Loading Editor Lite on demand is no different from loading any other module: ```javascript someActionFunction() { @@ -109,8 +109,8 @@ which would not depend on any particular group. Even though the Editor Lite's co [Create::Editor FE Team](https://about.gitlab.com/handbook/engineering/development/dev/create-editor/), the main functional elements — extensions — can be owned by any group. Editor Lite extensions' main idea is that the core of the editor remains very slim and stable. At the same time, whatever new functionality -is needed can be added as an extension to this core, without touching the core itself. It allows any group -to build and own any new editing functionality without being afraid of it being broken or overridden with +is needed can be added as an extension to this core, without touching the core itself. Any group is allowed +to build and own new editing functionality without being afraid of it being broken or overridden with the Editor Lite changes. Structurally, the complete implementation of Editor Lite could be presented as the following diagram: @@ -145,7 +145,7 @@ Important things to note here: ### Using an existing extension -Adding an extension to Editor Lite's instance is simple: +Adding an extension to Editor Lite's instance requires the following steps: ```javascript import EditorLite from '~/editor/editor_lite'; @@ -159,7 +159,7 @@ editor.use(MyExtension); ### Creating an extension -Let's create our first Editor Lite extension. As aforementioned, extensions are ES6 modules exporting the simple `Object` that is used to extend Editor Lite's functionality. As the most straightforward test, let's create an extension that extends Editor Lite with a new function that, when called, will output editor's content in `alert`. +Let's create our first Editor Lite extension. Extensions are ES6 modules exporting a basic `Object` that is used to extend Editor Lite's functionality. As a test, let's create an extension that extends Editor Lite with a new function that, when called, outputs editor's content in `alert`. `~/my_folder/my_fancy_extension.js:` @@ -225,7 +225,3 @@ Just pass the array of extensions to your `use` method: ```javascript editor.use([FileTemplateExtension, MyFancyExtension]); ``` - -## `` Vue component - -TBD diff --git a/doc/development/fe_guide/emojis.md b/doc/development/fe_guide/emojis.md index 7151e2ffeee..2dedbc8f19d 100644 --- a/doc/development/fe_guide/emojis.md +++ b/doc/development/fe_guide/emojis.md @@ -25,7 +25,7 @@ when your platform does not support it. - `app/assets/images/emoji.png` - `app/assets/images/emoji@2x.png` 1. Ensure you see new individual images copied into `app/assets/images/emoji/` - 1. Ensure you can see the new emojis and their aliases in the GFM Autocomplete + 1. Ensure you can see the new emojis and their aliases in the GitLab Flavored Markdown (GFM) Autocomplete 1. Ensure you can see the new emojis and their aliases in the award emoji menu 1. You might need to add new emoji Unicode support checks and rules for platforms that do not support a certain emoji and we need to fallback to an image. diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 9612f604b56..bf1dae6e7bd 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -21,7 +21,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## FAQ -### 1. How do I find the Rails route for a page? +### 1. How does one find the Rails route for a page? #### Check the 'page' data attribute @@ -36,7 +36,7 @@ Find here the [source code setting the attribute](https://gitlab.com/gitlab-org/ #### Rails routes -The `rake routes` command can be used to list all the routes available in the application, piping the output into `grep`, we can perform a search through the list of available routes. +The `rake routes` command can be used to list all the routes available in the application. Piping the output into `grep`, we can perform a search through the list of available routes. The output includes the request types available, route parameters and the relevant controller. ```shell @@ -46,13 +46,13 @@ bundle exec rake routes | grep "issues" ### 2. `modal_copy_button` vs `clipboard_button` The `clipboard_button` uses the `copy_to_clipboard.js` behavior, which is -initialized on page load, so if there are vue-based clipboard buttons that -don't exist at page load (such as ones in a `GlModal`), they do not have the +initialized on page load. Vue clipboard buttons that +don't exist at page load (such as ones in a `GlModal`) do not have click handlers associated with the clipboard package. -`modal_copy_button` was added that manages an instance of the +`modal_copy_button` manages an instance of the [`clipboard` plugin](https://www.npmjs.com/package/clipboard) specific to -the instance of that component, which means that clipboard events are +the instance of that component. This means that clipboard events are bound on mounting and destroyed when the button is, mitigating the above issue. It also has bindings to a particular container or modal ID available, to work with the focus trap created by our GlModal. @@ -60,7 +60,7 @@ available, to work with the focus trap created by our GlModal. ### 3. A `gitlab-ui` component not conforming to [Pajamas Design System](https://design.gitlab.com/) Some [Pajamas Design System](https://design.gitlab.com/) components implemented in -`gitlab-ui` do not conform with the design system specs because they lack some +`gitlab-ui` do not conform with the design system specs. This is because they lack some planned features or are not correctly styled yet. In the Pajamas website, a banner on top of the component examples indicates that: @@ -77,18 +77,17 @@ It makes codebase unified and more comfortable to maintain/refactor in the futur Ensure a [Product Designer](https://about.gitlab.com/company/team/?department=ux-department) reviews the use of the non-conforming component as part of the MR review. Make a -follow up issue and attach it to the component implementation epic found within the +follow up issue and attach it to the component implementation epic found in the [Components of Pajamas Design System epic](https://gitlab.com/groups/gitlab-org/-/epics/973). ### 4. My submit form button becomes disabled after submitting -If you are using a submit button inside a form and you attach an `onSubmit` event listener on the form element, [this piece of code](https://gitlab.com/gitlab-org/gitlab/blob/794c247a910e2759ce9b401356432a38a4535d49/app/assets/javascripts/main.js#L225) adds a `disabled` class selector to the submit button when the form is submitted. -To avoid this behavior, add the class `js-no-auto-disable` to the button. +A Submit button inside of a form attaches an `onSubmit` event listener on the form element. [This code](https://gitlab.com/gitlab-org/gitlab/blob/794c247a910e2759ce9b401356432a38a4535d49/app/assets/javascripts/main.js#L225) adds a `disabled` class selector to the submit button when the form is submitted. To avoid this behavior, add the class `js-no-auto-disable` to the button. -### 5. Should I use a full URL (i.e. `gon.gitlab_url`) or a full path (i.e. `gon.relative_url_root`) when referencing backend endpoints? +### 5. Should one use a full URL (for example `gon.gitlab_url`) or a full path (for example `gon.relative_url_root`) when referencing backend endpoints? -It's preferred to use a **full path** over a **full URL** because the URL uses the hostname configured with -GitLab which may not match the request. This causes [CORS issues like this Web IDE one](https://gitlab.com/gitlab-org/gitlab/-/issues/36810). +It's preferred to use a **full path** over a **full URL**. This is because the URL uses the hostname configured with +GitLab which may not match the request. This causes [cross-origin resource sharing issues like this Web IDE example](https://gitlab.com/gitlab-org/gitlab/-/issues/36810). Example: @@ -117,7 +116,7 @@ Example: ### 6. How should the Frontend reference Backend paths? -We prefer not to add extra coupling by hardcoding paths. If possible, +We prefer not to add extra coupling by hard-coding paths. If possible, add these paths as data attributes to the DOM element being referenced in the JavaScript. Example: @@ -153,7 +152,7 @@ export const fetchFoos = ({ state }) => { }; ``` -### 7. How can I test the production build locally? +### 7. How can one test the production build locally? Sometimes it's necessary to test locally what the frontend production build would produce, to do so the steps are: @@ -161,7 +160,7 @@ Sometimes it's necessary to test locally what the frontend production build woul 1. Open `gitlab.yaml` located in your `gitlab` installation folder, scroll down to the `webpack` section and change `dev_server` to `enabled: false`. 1. Run `yarn webpack-prod && gdk restart rails-web`. -The production build takes a few minutes to be completed; any code changes at this point are +The production build takes a few minutes to be completed. Any code changes at this point are displayed only after executing the item 3 above again. To return to the normal development mode: @@ -176,8 +175,8 @@ To return to the normal development mode: > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/28837) in GitLab 12.8. GitLab has enabled the Babel `preset-env` option -[`useBuiltIns: 'usage'`](https://babeljs.io/docs/en/babel-preset-env#usebuiltins-usage), -which adds the appropriate `core-js` polyfills once for each JavaScript feature +[`useBuiltIns: 'usage'`](https://babeljs.io/docs/en/babel-preset-env#usebuiltins-usage). +This adds the appropriate `core-js` polyfills once for each JavaScript feature we're using that our target browsers don't support. You don't need to add `core-js` polyfills manually. @@ -199,3 +198,7 @@ To see what polyfills are being used: which polyfills are being loaded and where: ![Image of webpack report](img/webpack_report_v12_8.png) + +### 9. Why is my page broken in dark mode? + +See [dark mode docs](dark_mode.md) diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index cbaa648570c..a53d9fee029 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -18,34 +18,46 @@ info: "See the Technical Writers assigned to Development Guidelines: https://abo **GraphQL at GitLab**: -- [🎬 GitLab Unfiltered GraphQL playlist](https://www.youtube.com/watch?v=wHPKZBDMfxE&list=PL05JrBw4t0KpcjeHjaRMB7IGB2oDWyJzv) -- [🎬 GraphQL at GitLab: Deep Dive](../api_graphql_styleguide.md#deep-dive) (video) by Nick Thomas + + +- [GitLab Unfiltered GraphQL playlist](https://www.youtube.com/watch?v=wHPKZBDMfxE&list=PL05JrBw4t0KpcjeHjaRMB7IGB2oDWyJzv) +- [GraphQL at GitLab: Deep Dive](../api_graphql_styleguide.md#deep-dive) (video) by Nick Thomas - An overview of the history of GraphQL at GitLab (not frontend-specific) -- [🎬 GitLab Feature Walkthrough with GraphQL and Vue Apollo](https://www.youtube.com/watch?v=6yYp2zB7FrM) (video) by Natalia Tepluhina +- [GitLab Feature Walkthrough with GraphQL and Vue Apollo](https://www.youtube.com/watch?v=6yYp2zB7FrM) (video) by Natalia Tepluhina - A real-life example of implementing a frontend feature in GitLab using GraphQL -- [🎬 History of client-side GraphQL at GitLab](https://www.youtube.com/watch?v=mCKRJxvMnf0) (video) Illya Klymov and Natalia Tepluhina -- [🎬 From Vuex to Apollo](https://www.youtube.com/watch?v=9knwu87IfU8) (video) by Natalia Tepluhina - - A useful overview of when Apollo might be a better choice than Vuex, and how one could go about the transition +- [History of client-side GraphQL at GitLab](https://www.youtube.com/watch?v=mCKRJxvMnf0) (video) Illya Klymov and Natalia Tepluhina +- [From Vuex to Apollo](https://www.youtube.com/watch?v=9knwu87IfU8) (video) by Natalia Tepluhina + - An overview of when Apollo might be a better choice than Vuex, and how one could go about the transition - [🛠 Vuex -> Apollo Migration: a proof-of-concept project](https://gitlab.com/ntepluhina/vuex-to-apollo/blob/master/README.md) - A collection of examples that show the possible approaches for state management with Vue+GraphQL+(Vuex or Apollo) apps + + ### Libraries We use [Apollo](https://www.apollographql.com/) (specifically [Apollo Client](https://www.apollographql.com/docs/react/)) and [Vue Apollo](https://github.com/vuejs/vue-apollo) when using GraphQL for frontend development. -If you are using GraphQL within a Vue application, the [Usage in Vue](#usage-in-vue) section +If you are using GraphQL in a Vue application, the [Usage in Vue](#usage-in-vue) section can help you learn how to integrate Vue Apollo. For other use cases, check out the [Usage outside of Vue](#usage-outside-of-vue) section. + + We use [Immer](https://immerjs.github.io/immer/docs/introduction) for immutable cache updates; see [Immutability and cache updates](#immutability-and-cache-updates) for more information. + + ### Tooling + + - [Apollo Client Devtools](https://github.com/apollographql/apollo-client-devtools) + + #### [Apollo GraphQL VS Code extension](https://marketplace.visualstudio.com/items?itemName=apollographql.vscode-apollo) If you use VS Code, the Apollo GraphQL extension supports autocompletion in `.graphql` files. To set up @@ -76,7 +88,7 @@ Our GraphQL API can be explored via GraphiQL at your instance's where needed. You can check all existing queries and mutations on the right side -of GraphiQL in its **Documentation explorer**. It's also possible to +of GraphiQL in its **Documentation explorer**. You can also write queries and mutations directly on the left tab and check their execution by clicking **Execute query** button on the top left: @@ -93,8 +105,8 @@ Default client accepts two parameters: `resolvers` and `config`. - `resolvers` parameter is created to accept an object of resolvers for [local state management](#local-state-with-apollo) queries and mutations - `config` parameter takes an object of configuration settings: - `cacheConfig` field accepts an optional object of settings to [customize Apollo cache](https://www.apollographql.com/docs/react/caching/cache-configuration/#configuring-the-cache) - - `baseUrl` allows us to pass a URL for GraphQL endpoint different from our main endpoint (i.e.`${gon.relative_url_root}/api/graphql`) - - `assumeImmutableResults` (set to `false` by default) - this setting, when set to `true`, will assume that every single operation on updating Apollo Cache is immutable. It also sets `freezeResults` to `true`, so any attempt on mutating Apollo Cache will throw a console warning in development environment. Please ensure you're following the immutability pattern on cache update operations before setting this option to `true`. + - `baseUrl` allows us to pass a URL for GraphQL endpoint different from our main endpoint (for example, `${gon.relative_url_root}/api/graphql`) + - `assumeImmutableResults` (set to `false` by default) - this setting, when set to `true`, assumes that every single operation on updating Apollo Cache is immutable. It also sets `freezeResults` to `true`, so any attempt on mutating Apollo Cache throws a console warning in development environment. Please ensure you're following the immutability pattern on cache update operations before setting this option to `true`. - `fetchPolicy` determines how you want your component to interact with the Apollo cache. Defaults to "cache-first". ## GraphQL Queries @@ -139,7 +151,7 @@ fragment DesignItem on Design { ``` More about fragments: -[GraphQL Docs](https://graphql.org/learn/queries/#fragments) +[GraphQL documentation](https://graphql.org/learn/queries/#fragments) ## Global IDs @@ -157,12 +169,17 @@ const primaryKeyId = getIdFromGraphQLId(data.id); ## Immutability and cache updates -From Apollo version 3.0.0 all the cache updates need to be immutable; it needs to be replaced entirely +From Apollo version 3.0.0 all the cache updates need to be immutable. It needs to be replaced entirely with a **new and updated** object. -To facilitate the process of updating the cache and returning the new object we use the library [Immer](https://immerjs.github.io/immer/docs/introduction). + + +To facilitate the process of updating the cache and returning the new object we +use the library [Immer](https://immerjs.github.io/immer/docs/introduction). When possible, follow these conventions: + + - The updated cache is named `data`. - The original cache data is named `sourceData`. @@ -184,10 +201,10 @@ client.writeQuery({ ``` As shown in the code example by using `produce`, we can perform any kind of direct manipulation of the -`draftState`. Besides, `immer` guarantees that a new state which includes the changes to `draftState` will be generated. +`draftState`. Besides, `immer` guarantees that a new state which includes the changes to `draftState` is generated. Finally, to verify whether the immutable cache update is working properly, we need to change -`assumeImmutableResults` to `true` in the default client configuration (see [Apollo Client](#apollo-client) for more information). +`assumeImmutableResults` to `true` in the default client configuration. See [Apollo Client](#apollo-client) for more information. If everything is working properly `assumeImmutableResults` should remain set to `true`. @@ -259,11 +276,11 @@ query User { } ``` -Along with creating local data, we can also extend existing GraphQL types with `@client` fields. This is extremely useful when we need to mock an API responses for fields not yet added to our GraphQL API. +Along with creating local data, we can also extend existing GraphQL types with `@client` fields. This is extremely helpful when we need to mock an API response for fields not yet added to our GraphQL API. #### Mocking API response with local Apollo cache -Using local Apollo Cache is handy when we have a need to mock some GraphQL API responses, queries or mutations locally (e.g. when they're still not added to our actual API). +Using local Apollo Cache is helpful when we have a need to mock some GraphQL API responses, queries, or mutations locally (such as when they're still not added to our actual API). For example, we have a [fragment](#fragments) on `DesignVersion` used in our queries: @@ -274,7 +291,7 @@ fragment VersionListItem on DesignVersion { } ``` -We need to fetch also version author and the 'created at' property to display them in the versions dropdown but these changes are still not implemented in our API. We can change the existing fragment to get a mocked response for these new fields: +We also need to fetch the version author and the `created at` property to display in the versions dropdown. But, these changes are still not implemented in our API. We can change the existing fragment to get a mocked response for these new fields: ```javascript fragment VersionListItem on DesignVersion { @@ -288,7 +305,7 @@ fragment VersionListItem on DesignVersion { } ``` -Now Apollo will try to find a _resolver_ for every field marked with `@client` directive. Let's create a resolver for `DesignVersion` type (why `DesignVersion`? because our fragment was created on this type). +Now Apollo tries to find a _resolver_ for every field marked with `@client` directive. Let's create a resolver for `DesignVersion` type (why `DesignVersion`? because our fragment was created on this type). ```javascript // resolvers.js @@ -319,13 +336,13 @@ import resolvers from './graphql/resolvers'; const defaultClient = createDefaultClient(resolvers); ``` -For each attempt to fetch a version, our client will fetch `id` and `sha` from the remote API endpoint and will assign our hardcoded values to the `author` and `createdAt` version properties. With this data, frontend developers are able to work on their UI without being blocked by backend. When the actual response is added to the API, our custom local resolver can be removed and the only change to the query/fragment is to remove the `@client` directive. +For each attempt to fetch a version, our client fetches `id` and `sha` from the remote API endpoint. It then assigns our hardcoded values to the `author` and `createdAt` version properties. With this data, frontend developers are able to work on their UI without being blocked by backend. When the response is added to the API, our custom local resolver can be removed. The only change to the query/fragment is to remove the `@client` directive. Read more about local state management with Apollo in the [Vue Apollo documentation](https://vue-apollo.netlify.app/guide/local-state.html#local-state). ### Using with Vuex -When Apollo Client is used within Vuex and fetched data is stored in the Vuex store, there is no need to keep Apollo Client cache enabled. Otherwise we would have data from the API stored in two places - Vuex store and Apollo Client cache. With Apollo's default settings, a subsequent fetch from the GraphQL API could result in fetching data from Apollo cache (in the case where we have the same query and variables). To prevent this behavior, we need to disable Apollo Client cache by passing a valid `fetchPolicy` option to its constructor: +When the Apollo Client is used in Vuex and fetched data is stored in the Vuex store, the Apollo Client cache does not need to be enabled. Otherwise we would have data from the API stored in two places - Vuex store and Apollo Client cache. With Apollo's default settings, a subsequent fetch from the GraphQL API could result in fetching data from Apollo cache (in the case where we have the same query and variables). To prevent this behavior, we need to disable Apollo Client cache by passing a valid `fetchPolicy` option to its constructor: ```javascript import fetchPolicies from '~/graphql_shared/fetch_policy_constants'; @@ -338,11 +355,61 @@ export const gqClient = createGqClient( ); ``` -### Feature flags in queries +### Working on GraphQL-based features when frontend and backend are not in sync + +Any feature that requires GraphQL queries/mutations to be created or updated should be carefully +planned. Frontend and backend counterparts should agree on a schema that satisfies both client-side and +server-side requirements. This enables both departments to start implementing their parts without +blocking each other. + +Ideally, the backend implementation should be done prior to the frontend so that the client can +immediately start querying the API with minimal back and forth between departments. However, we +recognize that priorities don't always align. For the sake of iteration and +delivering work we're committed to, it might be necessary for the frontend to be implemented ahead +of the backend. + +#### Implementing frontend queries and mutations ahead of the backend -Sometimes it may be useful to have an entity in the GraphQL query behind a feature flag. -For example, when working on a feature where the backend has already been merged but the frontend -hasn't you might want to put the GraphQL entity behind a feature flag to allow for smaller +In such case, the frontend will define GraphQL schemas or fields that do not correspond to any +backend resolver yet. This is fine as long as the implementation is properly feature-flagged so it +does not translate to public-facing errors in the product. However, we do validate client-side +queries/mutations against the backend GraphQL schema with the `graphql-verify` CI job. +You must confirm your changes pass the validation if they are to be merged before the +backend actually supports them. Below are a few suggestions to go about this. + +##### Using the `@client` directive + +The preferred approach is to use the `@client` directive on any new query, mutation, or field that +isn't yet supported by the backend. Any entity with the directive is skipped by the +`graphql-verify` validation job. + +Additionally Apollo will attempt to resolve them client-side, which can be used in conjunction with +[Mocking API response with local Apollo cache](#mocking-api-response-with-local-apollo-cache). This +provides a convenient way of testing your feature with fake data defined client-side. +When opening a merge request for your changes, it can be a good idea to provide local resolvers as a +patch that reviewers can apply in their GDK to easily smoke-test your work. + +Make sure to track the removal of the directive in a follow-up issue, or as part of the backend +implementation plan. + +##### Adding an exception to the list of known failures + +GraphQL queries/mutations validation can be completely turned off for specific files by adding their +paths to the +[`config/known_invalid_graphql_queries.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/config/known_invalid_graphql_queries.yml) +file, much like you would disable ESLint for some files via an `.eslintignore` file. +Bear in mind that any file listed in here will not be validated at all. So if you're only adding +fields to an existing query, use the `@client` directive approach so that the rest of the query +is still validated. + +Again, make sure that those overrides are as short-lived as possible by tracking their removal in +the appropriate issue. + +#### Feature flags in queries + +Sometimes it may be helpful to have an entity in the GraphQL query behind a feature flag. +One example is working on a feature where the backend has already been merged but the frontend +has not. In this case, you may consider putting the GraphQL entity behind a feature flag to allow smaller merge requests to be created and merged. To do this we can use the `@include` directive to exclude an entity if the `if` statement passes. @@ -355,7 +422,7 @@ query getAuthorData($authorNameEnabled: Boolean = false) { ``` Then in the Vue (or JavaScript) call to the query we can pass in our feature flag. This feature -flag will need to be already setup correctly. See the [feature flag documentation](../feature_flags/development.md) +flag needs to be already set up correctly. See the [feature flag documentation](../feature_flags/development.md) for the correct way to do this. ```javascript @@ -469,7 +536,7 @@ Note that we are using the [`pageInfo.fragment.graphql`](https://gitlab.com/gitl #### Using `fetchMore` method in components -This approach makes sense to use with user-handled pagination (e.g. when the scrolls to fetch more data or explicitly clicks a "Next Page"-button). +This approach makes sense to use with user-handled pagination. For example, when the scrolling to fetch more data or explicitly clicking a **Next Page** button. When we need to fetch all the data initially, it is recommended to use [a (non-smart) query, instead](#using-a-recursive-query-in-components). When making an initial fetch, we usually want to start a pagination from the beginning. @@ -479,7 +546,7 @@ In this case, we can either: - Pass `null` explicitly to `after`. After data is fetched, we can use the `update`-hook as an opportunity [to customize -the data that is set in the Vue component property](https://apollo.vuejs.org/api/smart-query.html#options), getting a hold of the `pageInfo` object among other data. +the data that is set in the Vue component property](https://apollo.vuejs.org/api/smart-query.html#options). This allows us to get a hold of the `pageInfo` object among other data. In the `result`-hook, we can inspect the `pageInfo` object to see if we need to fetch the next page. Note that we also keep a `requestCount` to ensure that the application @@ -561,8 +628,8 @@ fetchNextPage(endCursor) { When it is necessary to fetch all paginated data initially an Apollo query can do the trick for us. If we need to fetch the next page based on user interactions, it is recommend to use a [`smartQuery`](https://apollo.vuejs.org/api/smart-query.html) along with the [`fetchMore`-hook](#using-fetchmore-method-in-components). -When the query resolves we can update the component data and inspect the `pageInfo` object -to see if we need to fetch the next page, i.e. call the method recursively. +When the query resolves we can update the component data and inspect the `pageInfo` object. This allows us +to see if we need to fetch the next page, calling the method recursively. Note that we also keep a `requestCount` to ensure that the application does not keep requesting the next page, indefinitely. @@ -634,7 +701,7 @@ or [`.writeQuery()`](https://www.apollographql.com/docs/react/v2/api/apollo-clie This can be tedious and counter-intuitive. To make it easier to deal with cached paginated queries, Apollo provides the `@connection` directive. -The directive accepts a `key` parameter that will be used as a static key when caching the data. +The directive accepts a `key` parameter that is used as a static key when caching the data. You'd then be able to retrieve the data without providing any pagination-specific variables. Here's an example of a query using the `@connection` directive: @@ -661,7 +728,7 @@ query DastSiteProfiles($fullPath: ID!, $after: String, $before: String, $first: } ``` -In this example, Apollo will store the data with the stable `dastSiteProfiles` cache key. +In this example, Apollo stores the data with the stable `dastSiteProfiles` cache key. To retrieve that data from the cache, you'd then only need to provide the `$fullPath` variable, omitting pagination-specific variables like `after` or `before`: @@ -679,9 +746,9 @@ Read more about the `@connection` directive in [Apollo's documentation](https:// ### Managing performance -The Apollo client will batch queries by default. This means that if you have 3 queries defined, -Apollo will group them into one request, send the single request off to the server and only -respond once all 3 queries have completed. +The Apollo client batches queries by default. Given 3 deferred queries, +Apollo groups them into one request, sends the single request to the server, and +responds after all 3 queries have completed. If you need to have queries sent as individual requests, additional context can be provided to tell Apollo to do this. @@ -703,9 +770,13 @@ export default { #### Mocking response as component data -With [Vue test utils](https://vue-test-utils.vuejs.org/) it is easy to quickly test components that + + +With [Vue Test Utils](https://vue-test-utils.vuejs.org/) one can quickly test components that fetch GraphQL queries. The simplest way is to use `shallowMount` and then set -the data on the component +the data on the component: + + ```javascript it('tests apollo component', () => { @@ -719,7 +790,7 @@ it('tests apollo component', () => { #### Testing loading state -If we need to test how our component renders when results from the GraphQL API are still loading, we can mock a loading state into respective Apollo queries/mutations: +To test how a component renders when results from the GraphQL API are still loading, mock a loading state into respective Apollo queries/mutations: ```javascript function createComponent({ @@ -817,9 +888,9 @@ it('calls mutation on submitting form ', () => { To test the logic of Apollo cache updates, we might want to mock an Apollo Client in our unit tests. We use [`mock-apollo-client`](https://www.npmjs.com/package/mock-apollo-client) library to mock Apollo client and [`createMockApollo` helper](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/frontend/__helpers__/mock_apollo_helper.js) we created on top of it. -To separate tests with mocked client from 'usual' unit tests, it's recommended to create an additional factory and pass the created `mockApollo` as an option to the `createComponent`-factory. This way we only create Apollo Client instance when it's necessary. +To separate tests with mocked client from 'usual' unit tests, create an additional factory and pass the created `mockApollo` as an option to the `createComponent`-factory. This way we only create Apollo Client instance when it's necessary. -We need to inject `VueApollo` to the Vue local instance and, likewise, it is recommended to call `localVue.use()` within `createMockApolloProvider()` to only load it when it is necessary. +We need to inject `VueApollo` to the Vue local instance and, likewise, it is recommended to call `localVue.use()` in `createMockApolloProvider()` to only load it when it is necessary. ```javascript import VueApollo from 'vue-apollo'; @@ -861,7 +932,7 @@ describe('Some component', () => { }); ``` -Within `createMockApolloProvider`-factory, we need to define an array of _handlers_ for every query or mutation: +In the `createMockApolloProvider`-factory, we need to define an array of _handlers_ for every query or mutation: ```javascript import getDesignListQuery from '~/design_management/graphql/queries/get_design_list.query.graphql'; @@ -1251,9 +1322,9 @@ describe('My Index test with `createMockApollo`', () => { ## Handling errors -The GitLab GraphQL mutations currently have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). +The GitLab GraphQL mutations have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). -When utilising a GraphQL mutation, we must consider handling **both of these error modes** to ensure that the user receives the appropriate feedback when an error occurs. +When utilising a GraphQL mutation, consider handling **both of these error modes** to ensure that the user receives the appropriate feedback when an error occurs. ### Top-level errors @@ -1261,13 +1332,13 @@ These errors are located at the "top level" of a GraphQL response. These are non #### Handling top-level errors -Apollo is aware of top-level errors, so we are able to leverage Apollo's various error-handling mechanisms to handle these errors (e.g. handling Promise rejections after invoking the [`mutate`](https://www.apollographql.com/docs/react/api/core/ApolloClient/#ApolloClient.mutate) method, or handling the `error` event emitted from the [`ApolloMutation`](https://apollo.vuejs.org/api/apollo-mutation.html#events) component). +Apollo is aware of top-level errors, so we are able to leverage Apollo's various error-handling mechanisms to handle these errors. For example, handling Promise rejections after invoking the [`mutate`](https://www.apollographql.com/docs/react/api/core/ApolloClient/#ApolloClient.mutate) method, or handling the `error` event emitted from the [`ApolloMutation`](https://apollo.vuejs.org/api/apollo-mutation.html#events) component. Because these errors are not intended for users, error messages for top-level errors should be defined client-side. ### Errors-as-data -These errors are nested within the `data` object of a GraphQL response. These are recoverable errors that, ideally, can be presented directly to the user. +These errors are nested in the `data` object of a GraphQL response. These are recoverable errors that, ideally, can be presented directly to the user. #### Handling errors-as-data @@ -1283,7 +1354,7 @@ mutation createNoteMutation($input: String!) { } ``` -Now, when we commit this mutation and errors occur, the response will include `errors` for us to handle: +Now, when we commit this mutation and errors occur, the response includes `errors` for us to handle: ```javascript { @@ -1316,7 +1387,7 @@ When [using Vuex](#using-with-vuex), disable the cache when: - The data is being cached elsewhere - The use case does not need caching -if the data is being cached elsewhere, or if there is simply no need for it for the given use case. +if the data is being cached elsewhere, or if there is no need for it for the given use case. ```javascript import createDefaultClient from '~/lib/graphql'; @@ -1416,7 +1487,7 @@ for your application. To add GraphQL startup calls, we use `add_page_startup_graphql_call` helper where the first parameter is a path to the query, the second one is an object containing query variables. Path to the query is relative to `app/graphql/queries` folder: for example, if we need a -`app/graphql/queries/repository/files.query.graphql` query, the path will be +`app/graphql/queries/repository/files.query.graphql` query, the path is `repository/files`. ```yaml diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index af587a31bbb..a7b62fbb267 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -26,11 +26,11 @@ To use a sprite Icon in HAML or Rails we use a specific helper function: sprite_icon(icon_name, size: nil, css_class: '') ``` -- **icon_name**: Use the icon_name for the SVG sprite in the list of +- **`icon_name`**: Use the `icon_name` for the SVG sprite in the list of ([GitLab SVGs](https://gitlab-org.gitlab.io/gitlab-svgs)). -- **size (optional)**: Use one of the following sizes : 16, 24, 32, 48, 72 (this +- **`size` (optional)**: Use one of the following sizes : 16, 24, 32, 48, 72 (this is translated into a `s16` class) -- **css_class (optional)**: If you want to add additional CSS classes. +- **`css_class` (optional)**: If you want to add additional CSS classes. **Example** @@ -100,7 +100,7 @@ by the examples that follow: - `container` (optional): wraps the loading icon in a container, which centers the loading icon using the `text-center` CSS property. - `color` (optional): either `orange` (default), `light`, or `dark`. - `size` (optional): either `sm` (default), `md`, `lg`, or `xl`. -- `css_class` (optional): defaults to an empty string, but can be useful for utility classes to fine-tune alignment or spacing. +- `css_class` (optional): defaults to an empty string, but can be used for utility classes to fine-tune alignment or spacing. **Example 1:** @@ -164,8 +164,8 @@ export default { ## SVG Illustrations -Please use from now on for any SVG based illustrations simple `img` tags to show an illustration by simply using either `image_tag` or `image_path` helpers. -Please use the class `svg-content` around it to ensure nice rendering. +From now on, use `img` tags to display any SVG based illustrations with either `image_tag` or `image_path` helpers. +Using the class `svg-content` around it ensures nice rendering. ### Usage in HAML/Rails diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 84c1623f8c0..711c6a5f875 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -11,8 +11,11 @@ across the GitLab frontend team. ## Overview -GitLab is built on top of [Ruby on Rails](https://rubyonrails.org) using [Haml](https://haml.info/) and also a JavaScript based Frontend with [Vue.js](https://vuejs.org). -Be wary of [the limitations that come with using Hamlit](https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations). We also use [SCSS](https://sass-lang.com) and plain JavaScript with +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org). It uses [Haml](https://haml.info/) and a JavaScript0based frontend with [Vue.js](https://vuejs.org). + +Be wary of [the limitations that come with using Hamlit](https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations). + +We also use [SCSS](https://sass-lang.com) and plain JavaScript with modern ECMAScript standards supported through [Babel](https://babeljs.io/) and ES module support through [webpack](https://webpack.js.org/). Working with our frontend assets requires Node (v10.13.0 or greater) and Yarn @@ -21,7 +24,7 @@ Working with our frontend assets requires Node (v10.13.0 or greater) and Yarn ### Browser Support -For our currently-supported browsers, see our [requirements](../../install/requirements.md#supported-web-browsers). +For supported browsers, see our [requirements](../../install/requirements.md#supported-web-browsers). Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. Sign in to BrowserStack with the credentials saved in the **Engineering** vault of the GitLab @@ -56,7 +59,11 @@ Reusable components with technical and usage guidelines can be found in our ## Design Patterns -Common JavaScript [design patterns](design_patterns.md) in the GitLab codebase. +JavaScript [design patterns](design_patterns.md) in the GitLab codebase. + +## Design Anti-patterns + +JavaScript [design anti-patterns](design_anti_patterns.md) we try to avoid. ## Vue.js Best Practices @@ -121,3 +128,7 @@ 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. + +## [Troubleshooting](troubleshooting.md) + +Running into a Frontend development problem? Check out [this guide](troubleshooting.md) to help resolve your issue. diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index aac2258f3a3..795de87d309 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -202,15 +202,15 @@ help identify marks and measures coming from the different apps on the same page ## Best Practices -### Realtime Components +### Real-time Components -When writing code for realtime features we have to keep a couple of things in mind: +When writing code for real-time features we have to keep a couple of things in mind: 1. Do not overload the server with requests. -1. It should feel realtime. +1. It should feel real-time. -Thus, we must strike a balance between sending requests and the feeling of realtime. -Use the following rules when creating realtime solutions. +Thus, we must strike a balance between sending requests and the feeling of real-time. +Use the following rules when creating real-time solutions. 1. The server tells you how much to poll by sending `Poll-Interval` in the header. Use that as your polling interval. This enables system administrators to change the @@ -219,9 +219,9 @@ Use the following rules when creating realtime solutions. 1. A response with HTTP status different from 2XX should disable polling as well. 1. Use a common library for polling. 1. Poll on active tabs only. Please use [Visibility](https://github.com/ai/visibilityjs). -1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval is +1. Use regular polling intervals, do not use backoff polling or jitter, as the interval is controlled by the server. -1. The backend code is likely to be using etags. You do not and should not check for status +1. The backend code is likely to be using ETags. You do not and should not check for status `304 Not Modified`. The browser transforms it for you. ### Lazy Loading Images @@ -230,12 +230,12 @@ To improve the time to first render we are using lazy loading for images. This w the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, the value of `data-src` is moved to `src` automatically if the image is in the current viewport. -- Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src` AND adding the class `lazy`. +- Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src` and adding the class `lazy`. - If you are using the Rails `image_tag` helper, all images are lazy-loaded by default unless `lazy: false` is provided. -If you are asynchronously adding content which contains lazy images then you need to call the function +When asynchronously adding content which contains lazy images, call the function `gl.lazyLoader.searchLazyImages()` which searches for lazy images and loads them if needed. -But in general it should be handled automatically through a `MutationObserver` in the lazy loading function. +In general, it should be handled automatically through a `MutationObserver` in the lazy loading function. ### Animations @@ -243,7 +243,7 @@ Only animate `opacity` & `transform` properties. Other properties (such as `top` Layout to be recalculated, which is much more expensive. For details on this, see "Styles that Affect Layout" in [High Performance Animations](https://www.html5rocks.com/en/tutorials/speed/high-performance-animations/). -If you _do_ need to change layout (for example, a sidebar that pushes main content over), prefer [FLIP](https://aerotwist.com/blog/flip-your-animations/) to change expensive +If you _do_ need to change layout (for example, a sidebar that pushes main content over), prefer [FLIP](https://aerotwist.com/blog/flip-your-animations/). FLIP allows you to change expensive properties once, and handle the actual animation with transforms. ## Reducing Asset Footprint @@ -251,7 +251,7 @@ properties once, and handle the actual animation with transforms. ### Universal code Code that is contained in `main.js` and `commons/index.js` is loaded and -run on _all_ pages. **DO NOT ADD** anything to these files unless it is truly +run on _all_ pages. **Do not add** anything to these files unless it is truly needed _everywhere_. These bundles include ubiquitous libraries like `vue`, `axios`, and `jQuery`, as well as code for the main navigation and sidebar. Where possible we should aim to remove modules from these bundles to reduce our @@ -277,9 +277,9 @@ 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. NOTE: -If you are unsure what controller and action corresponds to a given page, you -can find this out by inspecting `document.body.dataset.page` in your -browser's developer console while on any page in GitLab. +When unsure what controller and action corresponds to a page, +inspect `document.body.dataset.page` in your +browser's developer console from any page in GitLab. #### Important Considerations @@ -294,7 +294,7 @@ browser's developer console while on any page in GitLab. All GitLab JavaScript files are added with the `defer` attribute. According to the [Mozilla documentation](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer), this implies that "the script is meant to be executed after the document has - been parsed, but before firing `DOMContentLoaded`". Since the document is already + been parsed, but before firing `DOMContentLoaded`". Because the document is already parsed, `DOMContentLoaded` is not needed to bootstrap applications because all the DOM nodes are already at our disposal. @@ -333,7 +333,7 @@ browser's developer console while on any page in GitLab. action(); ``` - For example, see how we use this in [app/assets/javascripts/pages/projects/graphs/charts/index.js](https://gitlab.com/gitlab-org/gitlab/-/commit/5e90885d6afd4497002df55bf015b338efcfc3c5#02e81de37f5b1716a3ef3222fa7f7edf22c40969_9_8): + For example, see how we use this in [`app/assets/javascripts/pages/projects/graphs/charts/index.js`](https://gitlab.com/gitlab-org/gitlab/-/commit/5e90885d6afd4497002df55bf015b338efcfc3c5#02e81de37f5b1716a3ef3222fa7f7edf22c40969_9_8): ```javascript waitForCSSLoaded(() => { @@ -366,9 +366,9 @@ browser's developer console while on any page in GitLab. ### Code Splitting -For any code that does not need to be run immediately upon page load, (for example, -modals, dropdowns, and other behaviors that can be lazy-loaded), you can split -your module into asynchronous chunks with dynamic import statements. These +Code that does not need to be run immediately upon page load (for example, +modals, dropdowns, and other behaviors that can be lazy-loaded) should be split +into asynchronous chunks with dynamic import statements. These imports return a Promise which is resolved after the script has loaded: ```javascript @@ -377,16 +377,16 @@ import(/* webpackChunkName: 'emoji' */ '~/emoji') .catch(/* report error */) ``` -Please try to use `webpackChunkName` when generating these dynamic imports as +Use `webpackChunkName` when generating dynamic imports as it provides a deterministic filename for the chunk which can then be cached -the browser across GitLab versions. +in the browser across GitLab versions. More information is available in [webpack's code splitting documentation](https://webpack.js.org/guides/code-splitting/#dynamic-imports). ### Minimizing page size -A smaller page size means the page loads faster (especially important on mobile -and poor connections), the page is parsed more quickly by the browser, and less +A smaller page size means the page loads faster, especially on mobile +and poor connections. The page is parsed more quickly by the browser, and less data is used for users with capped data plans. General tips: diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md index 627c5f4d12f..df4613d521d 100644 --- a/doc/development/fe_guide/security.md +++ b/doc/development/fe_guide/security.md @@ -8,7 +8,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w ## Resources -[Mozilla’s HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) and the +[Mozilla’s HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) and [Qualys SSL Labs Server Test](https://www.ssllabs.com/ssltest/analyze.html) are good resources for finding potential problems and ensuring compliance with security best practices. @@ -76,7 +76,7 @@ such as with reCAPTCHA, which cannot be used without an `iframe`. In order to protect users from [XSS vulnerabilities](https://en.wikipedia.org/wiki/Cross-site_scripting), we intend to disable inline scripts in the future using Content Security Policy. -While inline scripts can be useful, they're also a security concern. If +While inline scripts can make something easier, they're also a security concern. If user-supplied content is unintentionally left un-sanitized, malicious users can inject scripts into the web app. diff --git a/doc/development/fe_guide/style/html.md b/doc/development/fe_guide/style/html.md index 7fedbc6ce0d..e53686de1a0 100644 --- a/doc/development/fe_guide/style/html.md +++ b/doc/development/fe_guide/style/html.md @@ -6,6 +6,38 @@ info: To determine the technical writer assigned to the Stage/Group associated w # HTML style guide +## Semantic elements + +[Semantic elements](https://developer.mozilla.org/en-US/docs/Glossary/Semantics) are HTML tags that +give semantic (rather than presentational) meaning to the data they contain. For example: + +- [`
`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article) +- [`