Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike Lewis <mlewis@gitlab.com>2019-02-12 00:20:33 +0300
committerMike Lewis <mlewis@gitlab.com>2019-02-12 00:20:33 +0300
commit181db283856fdcce657b695676403c9a2a159579 (patch)
treec35383043cf82c37539979516a3817fd3547e777 /doc/development
parent62f65a6d023509a33fbbcbe2f2682d03b838702c (diff)
parenta1215556ec6c5ab84862519b82cee99645dad357 (diff)
Merge branch 'master' into 'template-improvements-for-documentation'
# Conflicts: # .gitlab/issue_templates/Feature proposal.md
Diffstat (limited to 'doc/development')
-rw-r--r--doc/development/README.md4
-rw-r--r--doc/development/contributing/issue_workflow.md2
-rw-r--r--doc/development/contributing/merge_request_workflow.md8
-rw-r--r--doc/development/contributing/style_guides.md1
-rw-r--r--doc/development/documentation/index.md8
-rw-r--r--doc/development/ee_features.md14
-rw-r--r--doc/development/file_storage.md2
-rw-r--r--doc/development/go_guide/index.md216
-rw-r--r--doc/development/i18n/externalization.md6
-rw-r--r--doc/development/i18n/index.md2
-rw-r--r--doc/development/i18n/merging_translations.md2
-rw-r--r--doc/development/profiling.md10
-rw-r--r--doc/development/sql.md34
13 files changed, 272 insertions, 37 deletions
diff --git a/doc/development/README.md b/doc/development/README.md
index 05715274a81..d5829e31343 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -123,3 +123,7 @@ description: 'Learn how to contribute to GitLab.'
## Compliance
- [Licensing](licensing.md) for ensuring license compliance
+
+## Go guides
+
+- [Go Guidelines](go_guide/index.md)
diff --git a/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md
index 24feb1378a1..c5344139ab4 100644
--- a/doc/development/contributing/issue_workflow.md
+++ b/doc/development/contributing/issue_workflow.md
@@ -20,7 +20,7 @@ All labels, their meaning and priority are defined on the
If you come across an issue that has none of these, and you're allowed to set
labels, you can _always_ add the team and type, and often also the subject.
-[milestones-page]: https://gitlab.com/gitlab-org/gitlab-ce/milestones
+[milestones-page]: https://gitlab.com/groups/gitlab-org/-/milestones
## Type labels
diff --git a/doc/development/contributing/merge_request_workflow.md b/doc/development/contributing/merge_request_workflow.md
index 9bef0635e3f..4e766f37871 100644
--- a/doc/development/contributing/merge_request_workflow.md
+++ b/doc/development/contributing/merge_request_workflow.md
@@ -86,6 +86,14 @@ request is as follows:
guidelines](../merge_request_performance_guidelines.md).
1. For tests that use Capybara or PhantomJS, see this [article on how
to write reliable asynchronous tests](https://robots.thoughtbot.com/write-reliable-asynchronous-integration-tests-with-capybara).
+1. If your merge request introduces changes that require additional steps when
+ installing GitLab from source, add them to `doc/install/installation.md` in
+ the same merge request.
+1. If your merge request introduces changes that require additional steps when
+ upgrading GitLab from source, add them to
+ `doc/update/upgrading_from_source.md` in the same merge request. If these
+ instructions are specific to a version, add them to the "Version specific
+ upgrading instructions" section.
Please keep the change in a single MR **as small as possible**. If you want to
contribute a large feature think very hard what the minimum viable change is.
diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md
index 6f1ba5d62a5..0eedef5e14f 100644
--- a/doc/development/contributing/style_guides.md
+++ b/doc/development/contributing/style_guides.md
@@ -21,6 +21,7 @@
of _prohibited this user from being saved due to the following errors:_ the
text should be _sorry, we could not create your account because:_
1. Code should be written in [US English][us-english]
+1. [Go](../go_guide/index.md)
This is also the style used by linting tools such as
[RuboCop](https://github.com/bbatsov/rubocop) and [Hound CI](https://houndci.com).
diff --git a/doc/development/documentation/index.md b/doc/development/documentation/index.md
index 828f9bfeec6..287a472d0d8 100644
--- a/doc/development/documentation/index.md
+++ b/doc/development/documentation/index.md
@@ -12,7 +12,7 @@ In addition to this page, the following resources to help craft and contribute d
- [Structure and template](structure.md) - Learn the typical parts of a doc page and how to write each one.
- [Workflow](workflow.md) - A landing page for our key workflows:
- [Feature-change documentation workflow](feature-change-workflow.md) - Adding required documentation when developing a GitLab feature.
- - [Documentation improvement worflow](improvement-workflow.md) - New content not associated with a new feature.
+ - [Documentation improvement workflow](improvement-workflow.md) - New content not associated with a new feature.
- [Markdown Guide](https://about.gitlab.com/handbook/product/technical-writing/markdown-guide/) - A reference for the markdown implementation used by GitLab's documentation site and about.gitlab.com.
- [Site architecture](site_architecture/index.md) - How docs.gitlab.com is built.
@@ -27,7 +27,7 @@ The source of the documentation is maintained in the following repository locati
| Project | Path |
| --- | --- |
| [GitLab Community Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc) |
-| [GitLab Enterprise Edition](https://gitlab.com/gitlab-org/gitlab-ce/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) |
+| [GitLab Enterprise Edition](https://gitlab.com/gitlab-org/gitlab-ee/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) |
| [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/) | [`/docs`](https://gitlab.com/gitlab-org/gitlab-runner/tree/master/docs) |
| [Omnibus GitLab](https://gitlab.com/gitlab-org/omnibus-gitlab/) | [`/doc`](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc) |
@@ -78,7 +78,7 @@ you can immediately tell that you are navigating to user-related documentation
about project features; specifically about merge requests. Our site's paths match
those of our repository, so the clear structure also makes documentation easier to update.
-While the documentation is home to a variety of content types, we do not organize by content type.
+While the documentation is home to a variety of content types, we do not organize by content type.
For example, do not create groupings of similar media types (e.g. indexes of all articles, videos, etc.).
Similarly, we do not use glossaries or FAQs. Such grouping of content by type makes
it difficult to browse for the information you need and difficult to maintain up-to-date content.
@@ -498,7 +498,7 @@ If you want to know the in-depth details, here's what's really happening:
The following GitLab features are used among others:
-- [Manual actions](../../ci/yaml/README.md#manual-actions)
+- [Manual actions](../../ci/yaml/README.md#whenmanual)
- [Multi project pipelines](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html)
- [Review Apps](../../ci/review_apps/index.md)
- [Artifacts](../../ci/yaml/README.md#artifacts)
diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md
index 790b1bf951b..e0985922443 100644
--- a/doc/development/ee_features.md
+++ b/doc/development/ee_features.md
@@ -839,6 +839,20 @@ For example there can be an
`app/assets/javascripts/protected_branches/protected_branches_bundle.js` and an
EE counterpart
`ee/app/assets/javascripts/protected_branches/protected_branches_bundle.js`.
+The corresponding import statement would then look like this:
+
+```javascript
+// app/assets/javascripts/protected_branches/protected_branches_bundle.js
+import bundle from '~/protected_branches/protected_branches_bundle.js';
+
+// ee/app/assets/javascripts/protected_branches/protected_branches_bundle.js
+// (only works in EE)
+import bundle from 'ee/protected_branches/protected_branches_bundle.js';
+
+// in CE: app/assets/javascripts/protected_branches/protected_branches_bundle.js
+// in EE: ee/app/assets/javascripts/protected_branches/protected_branches_bundle.js
+import bundle from 'ee_else_ce/protected_branches/protected_branches_bundle.js';
+```
See the frontend guide [performance section](./fe_guide/performance.md) for
information on managing page-specific javascript within EE.
diff --git a/doc/development/file_storage.md b/doc/development/file_storage.md
index b90dc90e424..597812c8c49 100644
--- a/doc/development/file_storage.md
+++ b/doc/development/file_storage.md
@@ -18,6 +18,7 @@ There are many places where file uploading is used, according to contexts:
- Issues/MR/Notes Legacy Markdown attachments
- CI Artifacts (archive, metadata, trace)
- LFS Objects
+ - Merge request diffs
## Disk storage
@@ -37,6 +38,7 @@ they are still not 100% standardized. You can see them below:
| Issues/MR/Notes Legacy Markdown attachments | no | uploads/-/system/note/attachment/:id/:filename | `AttachmentUploader` | Note |
| CI Artifacts (CE) | yes | shared/artifacts/:disk_hash[0..1]/:disk_hash[2..3]/:disk_hash/:year_:month_:date/:job_id/:job_artifact_id (:disk_hash is SHA256 digest of project_id) | `JobArtifactUploader` | Ci::JobArtifact |
| LFS Objects (CE) | yes | shared/lfs-objects/:hex/:hex/:object_hash | `LfsObjectUploader` | LfsObject |
+| External merge request diffs | yes | shared/external-diffs/merge_request_diffs/mr-:parent_id/diff-:id | `ExternalDiffUploader` | MergeRequestDiff |
CI Artifacts and LFS Objects behave differently in CE and EE. In CE they inherit the `GitlabUploader`
while in EE they inherit the `ObjectStorage` and store files in and S3 API compatible object store.
diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md
new file mode 100644
index 00000000000..cdc806a2d31
--- /dev/null
+++ b/doc/development/go_guide/index.md
@@ -0,0 +1,216 @@
+# Go standards and style guidelines
+
+This document describes various guidelines and best practices for GitLab
+projects using the [Go language](https://golang.org).
+
+## Overview
+
+GitLab is built on top of [Ruby on Rails](https://rubyonrails.org/), but we're
+also using Go for projects where it makes sense. Go is a very powerful
+language, with many advantages, and is best suited for projects with a lot of
+IO (disk/network access), HTTP requests, parallel processing, etc. Since we
+have both Ruby on Rails and Go at GitLab, we should evaluate carefully which of
+the two is best for the job.
+
+This page aims to define and organize our Go guidelines, based on our various
+experiences. Several projects were started with different standards and they
+can still have specifics. They will be described in their respective
+`README.md` or `PROCESS.md` files.
+
+## Code Review
+
+We follow the common principles of
+[Go Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments).
+
+Reviewers and maintainers should pay attention to:
+
+- `defer` functions: ensure the presence when needed, and after `err` check.
+- Inject dependencies as parameters.
+- Void structs when marshalling to JSON (generates `null` instead of `[]`).
+
+### Security
+
+Security is our top priority at GitLab. During code reviews, we must take care
+of possible security breaches in our code:
+
+- XSS when using text/template
+- CSRF Protection using Gorilla
+- Use a Go version without known vulnerabilities
+- Don't leak secret tokens
+- SQL injections
+
+Remember to run
+[SAST](https://docs.gitlab.com/ee/user/project/merge_requests/sast.html)
+**[ULTIMATE]** on your project (or at least the [gosec
+analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/gosec)),
+and to follow our [Security
+requirements](../code_review.md#security-requirements).
+
+Web servers can take advantages of middlewares like [Secure](https://github.com/unrolled/secure).
+
+### Finding a reviewer
+
+Many of our projects are too small to have full-time maintainers. That's why we
+have a shared pool of Go reviewers at GitLab. To find a reviewer, use the
+[Engineering Projects](https://about.gitlab.com/handbook/engineering/projects/)
+page in the handbook. "GitLab Community Edition (CE)" and "GitLab Community
+Edition (EE)" both have a "Go" section with its list of reviewers.
+
+To add yourself to this list, add the following to your profile in the
+[team.yml](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/team.yml)
+file and ask your manager to review and merge.
+
+```yaml
+projects:
+ gitlab-ee: reviewer go
+ gitlab-ce: reviewer go
+```
+
+## Code style and format
+
+- Avoid global variables, even in packages. By doing so you will introduce side
+ effects if the package is included multiple times.
+- Use `go fmt` before committing ([Gofmt](https://golang.org/cmd/gofmt/) is a
+ tool that automatically formats Go source code).
+
+### Automatic linting
+
+All Go projects should include these GitLab CI/CD jobs:
+
+```yaml
+go lint:
+ image: golang:1.11
+ script:
+ - go get -u golang.org/x/lint/golint
+ - golint -set_exit_status
+```
+
+Once [recursive includes](https://gitlab.com/gitlab-org/gitlab-ce/issues/56836)
+become available, you will be able to share job templates like this
+[analyzer](https://gitlab.com/gitlab-org/security-products/ci-templates/raw/master/includes-dev/analyzer.yml).
+
+## Dependencies
+
+Dependencies should be kept to the minimum. The introduction of a new
+dependency should be argued in the merge request, as per our [Approval
+Guidelines](../code_review.html#approval-guidelines). Both [License
+Management](https://docs.gitlab.com/ee/user/project/merge_requests/license_management.html)
+**[ULTIMATE]** and [Dependency
+Scanning](https://docs.gitlab.com/ee/user/project/merge_requests/dependency_scanning.html)
+**[ULTIMATE]** should be activated on all projects to ensure new dependencies
+security status and license compatibility.
+
+### Modules
+
+Since Go 1.11, a standard dependency system is available behind the name [Go
+Modules](https://github.com/golang/go/wiki/Modules). It provides a way to
+define and lock dependencies for reproducible builds. It should be used
+whenever possible.
+
+There was a [bug on modules
+checksums](https://github.com/golang/go/issues/29278) in Go < v1.11.4, so make
+sure to use at least this version to avoid `checksum mismatch` errors.
+
+### ORM
+
+We don't use object-relational mapping libraries (ORMs) at GitLab (except
+[ActiveRecord](https://guides.rubyonrails.org/active_record_basics.html) in
+Ruby on Rails). Projects can be structured with services to avoid them.
+[PQ](https://github.com/lib/pq) should be enough to interact with PostgreSQL
+databases.
+
+### Migrations
+
+In the rare event of managing a hosted database, it's necessary to use a
+migration system like ActiveRecord is providing. A simple library like
+[Journey](https://github.com/db-journey/journey), designed to be used in
+`postgres` containers, can be deployed as long-running pods. New versions will
+deploy a new pod, migrating the data automatically.
+
+## Testing
+
+We should not use any specific library or framework for testing, as the
+[standard library](https://golang.org/pkg/) provides already everything to get
+started. For example, some external dependencies might be worth considering in
+case we decide to use a specific library or framework:
+
+- [Testify](https://github.com/stretchr/testify)
+- [httpexpect](https://github.com/gavv/httpexpect)
+
+Use [subtests](https://blog.golang.org/subtests) whenever possible to improve
+code readability and test output.
+
+### Benchmarks
+
+Programs handling a lot of IO or complex operations should always include
+[benchmarks](https://golang.org/pkg/testing/#hdr-Benchmarks), to ensure
+performance consistency over time.
+
+## CLIs
+
+Every Go program is launched from the command line.
+[cli](https://github.com/urfave/cli) is a convenient package to create command
+line apps. It should be used whether the project is a daemon or a simple cli
+tool. Flags can be mapped to [environment
+variables](https://github.com/urfave/cli#values-from-the-environment) directly,
+which documents and centralizes at the same time all the possible command line
+interactions with the program. Don't use `os.GetEnv`, it hides variables deep
+in the code.
+
+## Daemons
+
+### Logging
+
+The usage of a logging library is strongly recommended for daemons. Even though
+there is a `log` package in the standard library, we generally use
+[logrus](https://github.com/sirupsen/logrus). Its plugin ("hooks") system
+makes it a powerful logging library, with the ability to add notifiers and
+formatters at the logger level directly.
+
+### Tracing and Correlation
+
+[LabKit](https://gitlab.com/gitlab-org/labkit) is a place to keep common
+libraries for Go services. Currently it's vendored into two projects:
+Workhorse and Gitaly, and it exports two main (but related) pieces of
+functionality:
+
+- [`gitlab.com/gitlab-org/labkit/correlation`](https://gitlab.com/gitlab-org/labkit/tree/master/correlation):
+ for propagating and extracting correlation ids between services.
+- [`gitlab.com/gitlab-org/labkit/tracing`](https://gitlab.com/gitlab-org/labkit/tree/master/tracing):
+ for instrumenting Go libraries for distributed tracing.
+
+This gives us a thin abstraction over underlying implementations that is
+consistent across Workhorse, Gitaly, and, in future, other Go servers. For
+example, in the case of `gitlab.com/gitlab-org/labkit/tracing` we can switch
+from using Opentracing directly to using Zipkin or Gokit's own tracing wrapper
+without changes to the application code, while still keeping the same
+consistent configuration mechanism (i.e. the `GITLAB_TRACING` environment
+variable).
+
+### Context
+
+Since daemons are long-running applications, they should have mechanisms to
+manage cancellations, and avoid unnecessary resources consumption (which could
+lead to DDOS vulnerabilities). [Go
+Context](https://github.com/golang/go/wiki/CodeReviewComments#contexts) should
+be used in functions that can block and passed as the first parameter.
+
+## Dockerfiles
+
+Every project should have a `Dockerfile` at the root of their repository, to
+build and run the project. Since Go program are static binaries, they should
+not require any external dependency, and shells in the final image are useless.
+We encourage [Multistage
+builds](https://docs.docker.com/develop/develop-images/multistage-build/):
+
+- They let the user build the project with the right Go version and
+ dependencies.
+- They generate a small, self-contained image, derived from `Scratch`.
+
+Generated docker images should have the program at their `Entrypoint` to create
+portable commands. That way, anyone can run the image, and without parameters
+it will display its help message (if `cli` has been used).
+
+---
+
+[Return to Development documentation](../README.md).
diff --git a/doc/development/i18n/externalization.md b/doc/development/i18n/externalization.md
index 00db58a45a2..223585ebb55 100644
--- a/doc/development/i18n/externalization.md
+++ b/doc/development/i18n/externalization.md
@@ -231,11 +231,11 @@ This makes use of [`Intl.DateTimeFormat`].
- In Ruby/HAML, we have two ways of adding format to dates and times:
1. **Through the `l` helper**, i.e. `l(active_session.created_at, format: :short)`. We have some predefined formats for
-[dates](https://gitlab.com/gitlab-org/gitlab-ce/blob/v11.7.0/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab-ce/blob/v11.7.0/config/locales/en.yml#L261).
+ [dates](https://gitlab.com/gitlab-org/gitlab-ce/blob/v11.7.0/config/locales/en.yml#L54) and [times](https://gitlab.com/gitlab-org/gitlab-ce/blob/v11.7.0/config/locales/en.yml#L261).
If you need to add a new format, because other parts of the code could benefit from it,
you'll need to add it to [en.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/locales/en.yml) file.
- 2. **Through `strftime`**, i.e. `milestone.start_date.strftime('%b %-d')`. We use `strftime` in case none of the formats
- defined on [en.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/locales/en.yml) matches the date/time
+ 1. **Through `strftime`**, i.e. `milestone.start_date.strftime('%b %-d')`. We use `strftime` in case none of the formats
+ defined on [en.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/locales/en.yml) matches the date/time
specifications we need, and if there is no need to add it as a new format because is very particular (i.e. it's only used in a single view).
## Best practices
diff --git a/doc/development/i18n/index.md b/doc/development/i18n/index.md
index c44690a4c5d..5e4923341af 100644
--- a/doc/development/i18n/index.md
+++ b/doc/development/i18n/index.md
@@ -51,4 +51,4 @@ able to proofread and instructions on becoming a proofreader yourself.
Translations are typically included in the next major or minor release.
-See [Merging translations from Crowdin](merging_translations.md)
+See [Merging translations from Crowdin](merging_translations.md).
diff --git a/doc/development/i18n/merging_translations.md b/doc/development/i18n/merging_translations.md
index d172aa6da21..2fa7558d30b 100644
--- a/doc/development/i18n/merging_translations.md
+++ b/doc/development/i18n/merging_translations.md
@@ -4,7 +4,7 @@ Crowdin automatically syncs the `gitlab.pot` file presenting newly
added translations to the community of translators.
At the same time, it creates a merge request to merge all newly added
-& approved translations. Find the [merge reqeust created by
+& approved translations. Find the [merge request created by
`gitlab-crowdin-bot`](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests?scope=all&utf8=%E2%9C%93&state=opened&author_username=gitlab-crowdin-bot)
to see new and merged merge requests. They are created in EE and need
to be ported to CE manually.
diff --git a/doc/development/profiling.md b/doc/development/profiling.md
index 0b0c6dfc8cf..b7d9f640a3f 100644
--- a/doc/development/profiling.md
+++ b/doc/development/profiling.md
@@ -72,6 +72,16 @@ Gitlab::Profiler.print_by_total_time(result, max_percent: 60, min_percent: 2)
# 0.02 0.865 0.000 0.000 0.864 638 *Enumerable#inject
```
+To print the profile in HTML format, use the following example:
+
+
+```ruby
+result = Gitlab::Profiler.profile('/my-user')
+
+printer = RubyProf::CallStackPrinter.new(result)
+printer.print(File.open('/tmp/profile.html', 'w'))
+```
+
[GitLab-Profiler](https://gitlab.com/gitlab-com/gitlab-profiler) is a project
that builds on this to add some additional niceties, such as allowing
configuration with a single Yaml file for multiple URLs, and uploading of the
diff --git a/doc/development/sql.md b/doc/development/sql.md
index 06005a0a6f8..47519d39e74 100644
--- a/doc/development/sql.md
+++ b/doc/development/sql.md
@@ -256,32 +256,12 @@ violation, for example.
Using transactions does not solve this problem.
-The following pattern should be used to avoid the problem:
+To solve this we've added the `ApplicationRecord.safe_find_or_create_by`.
-```ruby
-Project.transaction do
- begin
- User.find_or_create_by(username: "foo")
- rescue ActiveRecord::RecordNotUnique
- retry
- end
-end
-```
-
-If the above block is run inside a transaction and hits the race
-condition, the transaction is aborted and we cannot simply retry (any
-further queries inside the aborted transaction are going to fail). We
-can employ [nested transactions](http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html#module-ActiveRecord::Transactions::ClassMethods-label-Nested+transactions)
-here to only rollback the "inner transaction". Note that `requires_new: true` is required here.
+This method can be used just as you would the normal
+`find_or_create_by` but it wraps the call in a *new* transaction and
+retries if it were to fail because of an
+`ActiveRecord::RecordNotUnique` error.
-```ruby
-Project.transaction do
- begin
- User.transaction(requires_new: true) do
- User.find_or_create_by(username: "foo")
- end
- rescue ActiveRecord::RecordNotUnique
- retry
- end
-end
-```
+To be able to use this method, make sure the model you want to use
+this on inherits from `ApplicationRecord`.