Commit 9c58409e authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'ce-to-ee-2018-11-28' into 'master'

CE upstream - 2018-11-28 12:21 UTC

Closes gitlab-runner#3320

See merge request gitlab-org/gitlab-ee!8621
parents 6b6e8755 ac273ede
...@@ -1821,13 +1821,6 @@ These variables can be later used in all executed commands and scripts. ...@@ -1821,13 +1821,6 @@ These variables can be later used in all executed commands and scripts.
The YAML-defined variables are also set to all created service containers, The YAML-defined variables are also set to all created service containers,
thus allowing to fine tune them. thus allowing to fine tune them.
To turn off global defined variables in a specific job, define an empty hash:
```yaml
job_name:
variables: {}
```
Except for the user defined variables, there are also the ones [set up by the Except for the user defined variables, there are also the ones [set up by the
Runner itself](../variables/README.md#predefined-variables-environment-variables). Runner itself](../variables/README.md#predefined-variables-environment-variables).
One example would be `CI_COMMIT_REF_NAME` which has the value of One example would be `CI_COMMIT_REF_NAME` which has the value of
......
...@@ -5,7 +5,7 @@ having your code reviewed. ...@@ -5,7 +5,7 @@ having your code reviewed.
All merge requests for GitLab CE and EE, whether written by a GitLab team member All merge requests for GitLab CE and EE, whether written by a GitLab team member
or a volunteer contributor, must go through a code review process to ensure the or a volunteer contributor, must go through a code review process to ensure the
code is effective, understandable, and maintainable. code is effective, understandable, maintainable, and secure.
## Getting your merge request reviewed, approved, and merged ## Getting your merge request reviewed, approved, and merged
...@@ -20,12 +20,24 @@ importance of involving reviewer(s) in the section on the responsibility of the ...@@ -20,12 +20,24 @@ importance of involving reviewer(s) in the section on the responsibility of the
If you need some guidance (e.g. it's your first merge request), feel free to ask If you need some guidance (e.g. it's your first merge request), feel free to ask
one of the [Merge request coaches][team]. one of the [Merge request coaches][team].
If you need assistance with security scans or comments, feel free to include the
Security Team (`@gitlab-com/gl-security`) in the review.
Depending on the areas your merge request touches, it must be **approved** by one Depending on the areas your merge request touches, it must be **approved** by one
or more [maintainers](https://about.gitlab.com/handbook/engineering/#maintainer): or more [maintainers](https://about.gitlab.com/handbook/engineering/#maintainer):
For approvals, we use the approval functionality found in the merge request For approvals, we use the approval functionality found in the merge request
widget. Reviewers can add their approval by [approving additionally](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#adding-or-removing-an-approval). widget. Reviewers can add their approval by [approving additionally](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#adding-or-removing-an-approval).
Getting your merge request **merged** also requires a maintainer. If it requires
more than one approval, the last maintainer to review and approve it will also merge it.
### Approval guidelines
As described in the section on the responsibility of the maintainer below, you
are recommended to get your merge request approved and merged by maintainer(s)
from teams other than your own.
1. If your merge request includes backend changes [^1], it must be 1. If your merge request includes backend changes [^1], it must be
**approved by a [backend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_backend)**. **approved by a [backend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab-ce_maintainers_backend)**.
1. If your merge request includes frontend changes [^1], it must be 1. If your merge request includes frontend changes [^1], it must be
...@@ -39,12 +51,12 @@ widget. Reviewers can add their approval by [approving additionally](https://doc ...@@ -39,12 +51,12 @@ widget. Reviewers can add their approval by [approving additionally](https://doc
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 filesystem change, it must be
**approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details. **approved by a [Distribution team member][team]**. See how to work with the [Distribution team](https://about.gitlab.com/handbook/engineering/dev-backend/distribution/) for more details.
Getting your merge request **merged** also requires a maintainer. If it requires #### Security requirements
more than one approval, the last maintainer to review and approve it will also merge it.
As described in the section on the responsibility of the maintainer below, you 1. If your merge request involves implementing, utilizing, or is otherwise related to any type of authentication, authorization, or session handling mechanism, it must be
are recommended to get your merge request approved and merged by maintainer(s) **approved by a [Security Engineer][team]**.
from other teams than your own. 1. If your merge request has a goal which requires a cryptographic function such as: confidentiality, integrity, authentication, or non-repudiation, it must be
**approved by a [Security Engineer][team]**.
### The responsibility of the merge request author ### The responsibility of the merge request author
...@@ -54,9 +66,10 @@ merge request author. ...@@ -54,9 +66,10 @@ merge request author.
Before assigning a merge request to a maintainer for approval and merge, they Before assigning a merge request to a maintainer for approval and merge, they
should be confident that it actually solves the problem it was meant to solve, should be confident that it actually solves the problem it was meant to solve,
that it does so in the most appropriate way, that it satisfies all requirements, that it does so in the most appropriate way, that it satisfies all requirements,
and that there are no remaining bugs, logical problems, or uncovered edge cases. and that there are no remaining bugs, logical problems, uncovered edge cases,
The merge request should also have a completed task list in its description and or known vulnerabilities. The merge request should also have a completed task
a passing CI pipeline to avoid unnecessary back and forth. list in its description and a passing CI pipeline to avoid unnecessary back and
forth.
To reach the required level of confidence in their solution, an author is expected To reach the required level of confidence in their solution, an author is expected
to involve other people in the investigation and implementation processes as to involve other people in the investigation and implementation processes as
...@@ -85,8 +98,8 @@ Since a maintainer's job only depends on their knowledge of the overall GitLab ...@@ -85,8 +98,8 @@ Since a maintainer's job only depends on their knowledge of the overall GitLab
codebase, and not that of any specific domain, they can review, approve and merge codebase, and not that of any specific domain, they can review, approve and merge
merge requests from any team and in any product area. merge requests from any team and in any product area.
In fact, authors are recommended to get their merge requests merged by maintainers In fact, authors are encouraged to get their merge requests merged by maintainers
from other teams than their own, to ensure that all code across GitLab is consistent from teams other than their own, to ensure that all code across GitLab is consistent
and can be easily understood by all contributors, from both inside and outside the and can be easily understood by all contributors, from both inside and outside the
company, without requiring team-specific expertise. company, without requiring team-specific expertise.
...@@ -103,6 +116,18 @@ as the maintainer to ultimately approve and merge it. ...@@ -103,6 +116,18 @@ as the maintainer to ultimately approve and merge it.
Maintainers should check before merging if the merge request is approved by the Maintainers should check before merging if the merge request is approved by the
required approvers. required approvers.
Maintainers must check before merging if the merge request is introducing new
vulnerabilities, by inspecting the list in the Merge Request [Security
Widget](https://docs.gitlab.com/ee/user/project/merge_requests/#security-reports-ultimate).
When in doubt, a [Security Engineer][team] can be involved. The list of detected
vulnerabilities must be either empty or containing:
- dismissed vulnerabilities in case of false positives
- vulnerabilities converted to issues
Maintainers should **never** dismiss vulnerabilities to "empty" the list,
without duly verifying them.
## Best practices ## Best practices
### Everyone ### Everyone
...@@ -153,7 +178,7 @@ first time. ...@@ -153,7 +178,7 @@ first time.
### Assigning a merge request for a review ### Assigning a merge request for a review
If you want to have your merge request reviewed you can assign it to any reviewer. The list of reviewers can be found on [Engineering projects](https://about.gitlab.com/handbook/engineering/projects/) page. If you want to have your merge request reviewed, you can assign it to any reviewer. The list of reviewers can be found on [Engineering projects](https://about.gitlab.com/handbook/engineering/projects/) page.
You can also use `ready for review` label. That means that your merge request is ready to be reviewed and any reviewer can pick it. It is recommended to use that label only if there isn't time pressure and make sure the merge request is assigned to a reviewer. You can also use `ready for review` label. That means that your merge request is ready to be reviewed and any reviewer can pick it. It is recommended to use that label only if there isn't time pressure and make sure the merge request is assigned to a reviewer.
...@@ -189,6 +214,9 @@ experience, refactors the existing code). Then: ...@@ -189,6 +214,9 @@ experience, refactors the existing code). Then:
subsequent revisions for anything that would be spotted after that. subsequent revisions for anything that would be spotted after that.
- Consider using the [Squash and - Consider using the [Squash and
merge][squash-and-merge] feature when the merge request has a lot of commits. merge][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.
[squash-and-merge]: https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html#squash-and-merge [squash-and-merge]: https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html#squash-and-merge
......
...@@ -629,10 +629,10 @@ ...@@ -629,10 +629,10 @@
eslint-plugin-promise "^4.0.1" eslint-plugin-promise "^4.0.1"
eslint-plugin-vue "^5.0.0-beta.3" eslint-plugin-vue "^5.0.0-beta.3"
"@gitlab/svgs@^1.38.0": "@gitlab/svgs@^1.40.0":
version "1.38.0" version "1.40.0"
resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.38.0.tgz#e2f6e73379d60c7c63af4df8242a94c4671a1dfe" resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.40.0.tgz#58d7545fde3a7af3c290b419d1de2405973e58c5"
integrity sha512-Mzv6PxVbWEPvvMgXHaGxk8UE1Gard2gifca6loLgfLH7BtjXfESiZyJdQkkTSeBYp5MoqQa88Kw+vJYobwjsSw== integrity sha512-Y5QkaZH5N84qSNSGPxaj+NNlI4kthUNet7eRS1QCnaskwcvuWd/vF0xYCPd/tbRnK9MIhkKzhbxatUYDZVgXTQ==
"@gitlab/ui@^1.11.0": "@gitlab/ui@^1.11.0":
version "1.11.0" version "1.11.0"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment