diff options
Diffstat (limited to 'doc/development')
| -rw-r--r-- | doc/development/README.md | 2 | ||||
| -rw-r--r-- | doc/development/code_review.md | 5 | ||||
| -rw-r--r-- | doc/development/contributing/design.md | 10 | ||||
| -rw-r--r-- | doc/development/diffs.md | 16 | ||||
| -rw-r--r-- | doc/development/documentation/index.md | 9 | ||||
| -rw-r--r-- | doc/development/documentation/styleguide.md | 80 | ||||
| -rw-r--r-- | doc/development/performance.md | 2 | ||||
| -rw-r--r-- | doc/development/permissions.md | 63 | ||||
| -rw-r--r-- | doc/development/prometheus_metrics.md | 48 |
9 files changed, 183 insertions, 52 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 20f8fa1d368..b37403552fe 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -47,6 +47,8 @@ description: 'Learn how to contribute to GitLab.' - [How to dump production data to staging](db_dump.md) - [Working with the GitHub importer](github_importer.md) - [Working with Merge Request diffs](diffs.md) +- [Permissions](permissions.md) +- [Prometheus metrics](prometheus_metrics.md) ## Performance guides diff --git a/doc/development/code_review.md b/doc/development/code_review.md index e50e6370c80..edf0b6f46df 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -27,8 +27,9 @@ There are a few rules to get your merge request accepted: ask or assign any [reviewers][projects] for a first review. 1. If you need some guidance (e.g. it's your first merge request), feel free to ask one of the [Merge request coaches][team]. - 1. The reviewer will assign the merge request to a maintainer once the - reviewer is satisfied with the state of the merge request. + 1. It is recommended that you assign a maintainer that is from a different team than your own. + This ensures that all code across GitLab is consistent and can be easily understood by all contributors. + 1. Keep in mind that maintainers are also going to perform a final code review. The ideal scenario is that the reviewer has already addressed any concerns the maintainer would have found, and the maintainer only has to perform the diff --git a/doc/development/contributing/design.md b/doc/development/contributing/design.md index 57ae318e821..45fe8c26591 100644 --- a/doc/development/contributing/design.md +++ b/doc/development/contributing/design.md @@ -25,10 +25,12 @@ There is a special type label called ~"product discovery". It represents a disco The initial issue should be about the problem we are solving. If a separate [product discovery issue](#product-discovery-issues) is needed for additional research and design work, it will be created by a PM or UX person. Assign the ~UX, ~"product discovery" and ~"Deliverable" labels, add a milestone and use a title that makes it clear that the scheduled issue is product discovery (e.g. `Product discovery for XYZ`). -When the ~"product discovery" issue has been completed, the UXer removes the ~UX -label, adds the ~"UX ready" label and closes the issue. This indicates the -UX work for the issue is complete. The UXer will also copy any designs to related -issues for implementation in an upcoming milestone. +In order to complete a product discovery issue in a release, you must complete the following: + +1. UXer removes the ~UX label, adds the ~"UX ready" label. +1. Modify the issue description in the product discovery issue to contain the final design. If it makes sense, the original information indicating the need for the design can be moved to a lower "Original Information" section. +1. Copy the design to the description of the delivery issue for which the product discovery issue was created. Do not simply refer to the product discovery issue as a separate source of truth. +1. In some cases, a product discovery issue also identifies future enhancements that will not go into the issue that originated the product discovery issue. For these items, create new issues containing the designs to ensure they are not lost. Put the issues in the backlog if they are agreed upon as good ideas. Otherwise leave them for triage. ## Style guides diff --git a/doc/development/diffs.md b/doc/development/diffs.md index 2738b1b5635..5e8e8cc7541 100644 --- a/doc/development/diffs.md +++ b/doc/development/diffs.md @@ -15,9 +15,9 @@ We're constantly moving Rugged calls to Gitaly and the progress can be followed When refreshing a Merge Request (pushing to a source branch, force-pushing to target branch, or if the target branch now contains any commits from the MR) we fetch the comparison information using `Gitlab::Git::Compare`, which fetches `base` and `head` data using Gitaly and diff between them through -`Gitlab::Git::Diff.between` (which uses _Gitaly_ if it's enabled, otherwise _Rugged_). +`Gitlab::Git::Diff.between`. The diffs fetching process _limits_ single file diff sizes and the overall size of the whole diff through a series of constant values. Raw diff files are -then persisted on `merge_request_diff_files` table. +then persisted on `merge_request_diff_files` table. Even though diffs higher than 10kb are collapsed (`Gitlab::Git::Diff::COLLAPSE_LIMIT`), we still keep them on Postgres. However, diff files over _safety limits_ (see the [Diff limits section](#diff-limits)) are _not_ persisted. @@ -63,34 +63,34 @@ File diffs will be collapsed (but be expandable) if 100 files have already been ```ruby -Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 +Gitlab::Git::DiffCollection.collection_limits[:safe_max_lines] = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines] = 5000 ``` File diffs will be collapsed (but be expandable) if 5000 lines have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes +Gitlab::Git::DiffCollection.collection_limits[:safe_max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:safe_max_files] * 5.kilobytes = 500.kilobytes ``` File diffs will be collapsed (but be expandable) if 500 kilobytes have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000 +Gitlab::Git::DiffCollection.collection_limits[:max_files] = Commit::DIFF_HARD_LIMIT_FILES = 1000 ``` No more files will be rendered at all if 1000 files have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000 +Gitlab::Git::DiffCollection.collection_limits[:max_lines] = Commit::DIFF_HARD_LIMIT_LINES = 50000 ``` No more files will be rendered at all if 50,000 lines have already been rendered. ```ruby -Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes +Gitlab::Git::DiffCollection.collection_limits[:max_bytes] = Gitlab::Git::DiffCollection.collection_limits[:max_files] * 5.kilobytes = 5000.kilobytes ``` No more files will be rendered at all if 5 megabytes have already been rendered. @@ -131,7 +131,7 @@ File diff will be suppressed (technically different from collapsed, but behaves ## Viewers Diff Viewers, which can be found on `models/diff_viewer/*` are classes used to map metadata about each type of Diff File. It has information -whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. +whether it's a binary, which partial should be used to render it or which File extensions this class accounts for. `DiffViewer::Base` validates _blobs_ (old and new versions) content, extension and file type in order to check if it can be rendered. diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md index f46c171d9f1..7ac211ed550 100644 --- a/doc/development/documentation/index.md +++ b/doc/development/documentation/index.md @@ -257,6 +257,15 @@ choices: If your branch name matches any of the above, it will run only the docs tests. If it doesn't, the whole test suite will run (including docs). +## Danger bot + +GitLab uses [danger bot](https://github.com/danger/danger) for some elements in +code review. For docs changes in merge requests, the following actions are taken: + +1. Whenever a change under `/doc` is made, the bot leaves a comment for the + author to mention `@gl-docsteam`, so that the docs can be properly + reviewed. + ## Merge requests for GitLab documentation Before getting started, make sure you read the introductory section diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index e610c7595a8..8083f219d4a 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -13,10 +13,10 @@ Check the GitLab handbook for the [writing styles guidelines](https://about.gitl ## Files - [Directory structure](index.md#location-and-naming-documents): place the docs -in the correct location -- [Documentation files](index.md#documentation-files): name the files accordingly +in the correct location. +- [Documentation files](index.md#documentation-files): name the files accordingly. - [Markdown](../../user/markdown.md): use the GitLab Flavored Markdown in the -documentation +documentation. NOTE: **Note:** **Do not** use capital letters, spaces, or special chars in file names, @@ -30,17 +30,17 @@ a test that will fail if it spots a new `README.md` file. - Split up long lines (wrap text), this makes it much easier to review and edit. Only double line breaks are shown as a full line break in [GitLab markdown][gfm]. - 80-100 characters is a good line length + 80-100 characters is a good line length. - Make sure that the documentation is added in the correct [directory](index.md#documentation-directory-structure) and that - there's a link to it somewhere useful -- Do not duplicate information -- Be brief and clear -- Unless there's a logical reason not to, add documents in alphabetical order -- Write in US English -- Use [single spaces][] instead of double spaces + there's a link to it somewhere useful. +- Do not duplicate information. +- Be brief and clear. +- Unless there's a logical reason not to, add documents in alphabetical order. +- Write in US English. +- Use [single spaces][] instead of double spaces. - Jump a line between different markups (e.g., after every paragraph, header, list, etc) -- Capitalize "G" and "L" in GitLab +- Capitalize "G" and "L" in GitLab. - Use sentence case for titles, headings, labels, menu items, and buttons. - Use title case when referring to [features](https://about.gitlab.com/features/) or [products](https://about.gitlab.com/pricing/) (e.g., GitLab Runner, Geo, @@ -50,10 +50,9 @@ some features are also objects (e.g. "Merge Requests" and "merge requests"). ## Formatting -- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`) -- Use undescore (`_`) for text in italics (`_italic_`) -- Jump a line between different markups, for example: - +- Use double asterisks (`**`) to mark a word or text in bold (`**bold**`). +- Use undescore (`_`) for text in italics (`_italic_`). +- Put an empty line between different markups. For example: ```md ## Header @@ -69,9 +68,16 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl ### Ordered and unordered lists -- Use dashes (`-`) for unordered lists instead of asterisks (`*`) -- Use the number one (`1`) for ordered lists -- For punctuation in bullet lists, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/) +- Use dashes (`-`) for unordered lists instead of asterisks (`*`). +- Use the number one (`1`) for ordered lists. +- Separate list items from explanatory text with a colon (`:`). For example: + ```md + The list is as follows: + + - First item: This explains the first item. + - Second item: This explains the second item. + ``` +- For further guidance on punctuation in bullet lists, please refer to the [GitLab UX guide](https://design.gitlab.com/content/punctuation/). ## Headings @@ -82,7 +88,7 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl - Avoid putting numbers in headings. Numbers shift, hence documentation anchor links shift too, which eventually leads to dead links. If you think it is compelling to add numbers in headings, make sure to at least discuss it with - someone in the Merge Request + someone in the Merge Request. - [Avoid using symbols and special chars](https://gitlab.com/gitlab-com/gitlab-docs/issues/84) in headers. Whenever possible, they should be plain and short text. - Avoid adding things that show ephemeral statuses. For example, if a feature is @@ -92,8 +98,8 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl of the following GitLab members for a review: `@axil` or `@marcia`. This is to ensure that no document with wrong heading is going live without an audit, thus preventing dead links and redirection issues when - corrected -- Leave exactly one new line after a heading + corrected. +- Leave exactly one new line after a heading. ## Links @@ -120,11 +126,11 @@ For punctuation rules, please refer to the [GitLab UX guide](https://design.gitl To indicate the steps of navigation through the UI: -- Use the exact word as shown in the UI, including any capital letters as-is +- Use the exact word as shown in the UI, including any capital letters as-is. - Use bold text for navigation items and the char `>` as separator -(e.g., `Navigate to your project's **Settings > CI/CD**` ) +(e.g., `Navigate to your project's **Settings > CI/CD**` ). - If there are any expandable menus, make sure to mention that the user -needs to expand the tab to find the settings you're referring to +needs to expand the tab to find the settings you're referring to. ## Images @@ -149,12 +155,12 @@ Inside the document: `` - Always use a proper description for what the image is about. That way, when a browser fails to show the image, this text will be used as an alternative - description + description. - If there are consecutive images with little text between them, always add three dashes (`---`) between the image and the text to create a horizontal - line for better clarity + line for better clarity. - If a heading is placed right after an image, always add three dashes (`---`) - between the image and the heading + between the image and the heading. ## Alert boxes @@ -262,18 +268,18 @@ below. When a feature is available in EE-only tiers, add the corresponding tier according to the feature availability: -- For GitLab Starter and GitLab.com Bronze: `**[STARTER]**` -- For GitLab Premium and GitLab.com Silver: `**[PREMIUM]**` -- For GitLab Ultimate and GitLab.com Gold: `**[ULTIMATE]**` -- For GitLab Core and GitLab.com Free: `**[CORE]**` +- For GitLab Starter and GitLab.com Bronze: `**[STARTER]**`. +- For GitLab Premium and GitLab.com Silver: `**[PREMIUM]**`. +- For GitLab Ultimate and GitLab.com Gold: `**[ULTIMATE]**`. +- For GitLab Core and GitLab.com Free: `**[CORE]**`. To exclude GitLab.com tiers (when the feature is not available in GitLab.com), add the keyword "only": -- For GitLab Starter: `**[STARTER ONLY]**` -- For GitLab Premium: `**[PREMIUM ONLY]**` -- For GitLab Ultimate: `**[ULTIMATE ONLY]**` -- For GitLab Core: `**[CORE ONLY]**` +- For GitLab Starter: `**[STARTER ONLY]**`. +- For GitLab Premium: `**[PREMIUM ONLY]**`. +- For GitLab Ultimate: `**[ULTIMATE ONLY]**`. +- For GitLab Core: `**[CORE ONLY]**`. The tier should be ideally added to headers, so that the full badge will be displayed. But it can be also mentioned from paragraphs, list items, and table cells. For these cases, @@ -328,8 +334,8 @@ prefer to document it in the CE docs to avoid duplication. Configuration settings include: -- settings that touch configuration files in `config/` -- NGINX settings and settings in `lib/support/` in general +- Settings that touch configuration files in `config/`. +- NGINX settings and settings in `lib/support/` in general. When there is a list of steps to perform, usually that entails editing the configuration file and reconfiguring/restarting GitLab. In such case, follow diff --git a/doc/development/performance.md b/doc/development/performance.md index 6b4cb6d72d1..05caffb150a 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -43,7 +43,7 @@ GitLab provides built-in tools to aid the process of improving performance: * [QueryRecoder](query_recorder.md) for preventing `N+1` regressions GitLab employees can use GitLab.com's performance monitoring systems located at -<http://performance.gitlab.net>, this requires you to log in using your +<https://dashboards.gitlab.net>, this requires you to log in using your `@gitlab.com` Email address. Non-GitLab employees are advised to set up their own InfluxDB + Grafana stack. diff --git a/doc/development/permissions.md b/doc/development/permissions.md new file mode 100644 index 00000000000..5d409c9461e --- /dev/null +++ b/doc/development/permissions.md @@ -0,0 +1,63 @@ +# GitLab permissions guide + +There are multiple types of permissions across GitLab, and when implementing +anything that deals with permissions, all of them should be considered. + +## Groups and Projects + +### General permissions + +Groups and projects can have the following visibility levels: + +- public (20) - an entity is visible to everyone +- internal (10) - an entity is visible to logged in users +- private (0) - an entity is visible only to the approved members of the entity + +The visibility level of a group can be changed only if all subgroups and +subprojects have the same or lower visibility level. (e.g., a group can be set +to internal only if all subgroups and projects are internal or private). + +Visibility levels can be found in the `Gitlab::VisibilityLevel` module. + +### Feature specific permissions + +Additionally, the following project features can have different visibility levels: + +- Issues +- Repository + - Merge Request + - Pipelines + - Container Registry + - Git Large File Storage +- Wiki +- Snippets + +These features can be set to "Everyone with Access" or "Only Project Members". +They make sense only for public or internal projects because private projects +can be accessed only by project members by default. + +### Members + +Users can be members of multiple groups and projects. The following access +levels are available (defined in the `Gitlab::Access` module): + +- Guest +- Reporter +- Developer +- Maintainer +- Owner + +If a user is the member of both a project and the project parent group, the +higher permission is taken into account for the project. + +If a user is the member of a project, but not the parent group (or groups), they +can still view the groups and their entities (like epics). + +Project membership (where the group membership is already taken into account) +is stored in the `project_authorizations` table. + +### Confidential issues + +Confidential issues can be accessed only by project members who are at least +reporters (they can't be accessed by guests). Additionally they can be accessed +by their authors and assignees. diff --git a/doc/development/prometheus_metrics.md b/doc/development/prometheus_metrics.md new file mode 100644 index 00000000000..b6b6d9665ea --- /dev/null +++ b/doc/development/prometheus_metrics.md @@ -0,0 +1,48 @@ +# Working with Prometheus Metrics + +## Adding to the library + +We strive to support the 2-4 most important metrics for each common system service that supports Prometheus. If you are looking for support for a particular exporter which has not yet been added to the library, additions can be made [to the `common_metrics.yml`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/prometheus/common_metrics.yml) file. + +### Query identifier + +The requirement for adding a new metric is to make each query to have an unique identifier which is used to update the metric later when changed: + +```yaml +- group: Response metrics (NGINX Ingress) + metrics: + - title: "Throughput" + y_label: "Requests / Sec" + queries: + - id: response_metrics_nginx_ingress_throughput_status_code + query_range: 'sum(rate(nginx_upstream_responses_total{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) by (status_code)' + unit: req / sec + label: Status Code +``` + +### Update existing metrics + +After you add or change existing _common_ metric you have to create a new database migration that will query and update all existing metrics. + +NOTE: **Note:** +If a query metric (which is identified by `id:`) is removed it will not be removed from database by default. +You might want to add additional database migration that makes a decision what to do with removed one. +For example: you might be interested in migrating all dependent data to a different metric. + +```ruby +class ImportCommonMetrics < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + require Rails.root.join('db/importers/common_metrics_importer.rb') + + DOWNTIME = false + + def up + Importers::CommonMetricsImporter.new.execute + end + + def down + # no-op + end +end +``` |