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)
with [domain expertise](#domain-experts).
with [domain expertise](#domain-experts).
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_maintainers_backend)**.
1. If your merge request includes database migrations or changes to expensive queries [^2], it must be
1. If your merge request includes database migrations or changes to expensive queries (*2*), it must be
**approved by a [database maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_database)**.
Read the [database review guidelines](database_review.md) for more details.
1. If your merge request includes frontend changes [^1], it must be
1. If your merge request includes frontend changes (*1*), it must be
**approved by a [frontend maintainer](https://about.gitlab.com/handbook/engineering/projects/#gitlab_maintainers_frontend)**.
1. If your merge request includes UX changes [^1], it must be
1. If your merge request includes UX changes (*1*), it must be
**approved by a [UX team member](https://about.gitlab.com/company/team/)**.
1. If your merge request includes adding a new JavaScript library [^1], it must be
1. If your merge request includes adding a new JavaScript library (*1*), it must be
**approved by a [frontend lead](https://about.gitlab.com/company/team/)**.
1. If your merge request includes adding a new UI/UX paradigm [^1], it must be
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
**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.
...
...
@@ -97,6 +97,11 @@ are recommended to get your merge request approved and merged by maintainer(s)
by a [Technical writer](https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers)**, based on
the appropriate [product category](https://about.gitlab.com/handbook/product/categories/).
- (*1*): Please note that specs other than JavaScript specs are considered backend code.
- (*2*): We encourage you to seek guidance from a database maintainer if your merge
request is potentially introducing expensive queries. It is most efficient to comment
on the line of code in question with the SQL queries so they can give their advice.
#### Security requirements
View the updated documentation regarding [internal application security reviews](https://about.gitlab.com/handbook/engineering/security/#internal-application-security-reviews) for **when** and **how** to request a security review.
...
...
@@ -495,6 +500,3 @@ Largely based on the [thoughtbot code review guide](https://github.com/thoughtbo
---
[Return to Development documentation](README.md)
[^1]:Please note that specs other than JavaScript specs are considered backend code.
[^2]:We encourage you to seek guidance from a database maintainer if your merge request is potentially introducing expensive queries. It is most efficient to comment on the line of code in question with the SQL queries so they can give their advice.
are useful for testing for potential accessibility problems in GitLab.
The [axe][axe-website] browser extension (available for [Firefox][axe-firefox-extension] and [Chrome][axe-chrome-extension]) is
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.
Accessibility best-practices and more in-depth information are available on
[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools. The "[awesome a11y][awesome-a11y]" list is also a
[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.
Read more about the [Apollo](https://www.apollographql.com/) client in the [Apollo documentation](https://www.apollographql.com/docs/tutorial/client/).
GitLab is built on top of [Ruby on Rails](https://rubyonrails.org) using [Haml][haml] and also a JavaScript based Frontend with [Vue.js](https://vuejs.org).
Be wary of [the limitations that come with using Hamlit][hamlit-limits]. We also use [SCSS](https://sass-lang.com) and plain JavaScript with
modern ECMAScript standards supported through [Babel][babel] and ES module support through [webpack][webpack].
GitLab is built on top of [Ruby on Rails](https://rubyonrails.org) using [Haml](http://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
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
(v1.10.0 or greater). You can find information on how to install these on our
For our currently-supported browsers, see our [requirements][requirements].
For our currently-supported browsers, see our [requirements](../../install/requirements.md#supported-web-browsers).
Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. Login to BrowserStack with the credentials saved in GitLab's [shared 1Password account](https://about.gitlab.com/handbook/security/#1password-for-teams).
...
...
@@ -83,7 +83,7 @@ Read the [frontend's FAQ](frontend_faq.md) for common small pieces of helpful in
See the relevant style guides for our guidelines and for information on linting:
-[JavaScript](style/javascript.md). Our guide is based on
the excellent [Airbnb][airbnb-js-style-guide] style guide with a few small
the excellent [Airbnb](https://github.com/airbnb/javascript) style guide with a few small
changes.
-[SCSS](style/scss.md): our SCSS conventions which are enforced through [`scss-lint`](https://github.com/sds/scss-lint).
-[HTML](style/html.md). Guidelines for writing HTML code consistent with the rest of the codebase.
...
...
@@ -109,14 +109,3 @@ 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.
If you _do_ need to change layout (e.g. a sidebar that pushes main content over), prefer [FLIP][flip] to change expensive
If you _do_ need to change layout (e.g. a sidebar that pushes main content over), prefer [FLIP](https://aerotwist.com/blog/flip-your-animations/) to change expensive
properties once, and handle the actual animation with transforms.
## Reducing Asset Footprint
...
...
@@ -160,18 +160,13 @@ General tips:
- If some functionality can reasonably be achieved without adding extra libraries, avoid them.
- Use page-specific JavaScript as described above to load libraries that are only needed on certain pages.
- Use code-splitting dynamic imports wherever possible to lazy-load code that is not needed initially.
[Mozilla’s HTTP Observatory CLI][observatory-cli] and the
[Qualys SSL Labs Server Test][qualys-ssl] are good resources for finding
[Mozilla’s HTTP Observatory CLI](https://github.com/mozilla/http-observatory-cli) and the
[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.
<!-- Uncomment these sections when CSP/SRI are implemented.
...
...
@@ -29,14 +29,14 @@ Some exceptions include:
- Connecting with GitHub, Bitbucket, GitLab.com, etc. to allow project importing.
- Connecting with Google, Twitter, GitHub, etc. to allow OAuth authentication.
We use [the Secure Headers gem][secure_headers] to enable Content
We use [the Secure Headers gem](https://github.com/twitter/secureheaders) to enable Content
Security Policy headers in the GitLab Rails app.
Some resources on implementing Content Security Policy:
-[MDN Article on CSP][mdn-csp]
-[GitHub’s CSP Journey on the GitHub Engineering Blog][github-eng-csp]
- The Dropbox Engineering Blog's series on CSP: [1][dropbox-csp-1], [2][dropbox-csp-2], [3][dropbox-csp-3], [4][dropbox-csp-4]
-[MDN Article on CSP](https://developer.mozilla.org/en-US/docs/Web/Security/CSP)
-[GitHub’s CSP Journey on the GitHub Engineering Blog](http://githubengineering.com/githubs-csp-journey/)
- The Dropbox Engineering Blog's series on CSP: [1](https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/), [2](https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/), [3](https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/), [4](https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/)
### Subresource Integrity (SRI)
...
...
@@ -52,8 +52,8 @@ All CSS and JavaScript assets should use Subresource Integrity.
Some resources on implementing Subresource Integrity:
-[MDN Article on SRI][mdn-sri]
-[Subresource Integrity on the GitHub Engineering Blog][github-eng-sri]
-[MDN Article on SRI](https://developer.mozilla.org/en-us/docs/web/security/subresource_integrity)
-[Subresource Integrity on the GitHub Engineering Blog](http://githubengineering.com/subresource-integrity/)
-->
...
...
@@ -67,7 +67,7 @@ such as with reCAPTCHA, which cannot be used without an `iframe`.
## Avoiding inline scripts and styles
In order to protect users from [XSS vulnerabilities][xss], we will disable
In order to protect users from [XSS vulnerabilities](https://en.wikipedia.org/wiki/Cross-site_scripting), we will disable
inline scripts in the future using Content Security Policy.
While inline scripts can be useful, they're also a security concern. If
...
...
@@ -77,16 +77,3 @@ inject scripts into the web app.
Inline styles should be avoided in almost all cases, they should only be used
when no alternatives can be found. This allows reusability of styles as well as
@@ -53,7 +53,7 @@ Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules)
## Naming
1.**Extensions**: Use `.vue` extension for Vue components. Do not use `.js` as file extension ([#34371]).
1.**Extensions**: Use `.vue` extension for Vue components. Do not use `.js` as file extension ([#34371](https://gitlab.com/gitlab-org/gitlab-foss/issues/34371)).
1.**Reference Naming**: Use PascalCase for their instances:
```javascript
...
...
@@ -89,8 +89,6 @@ Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules)
When there's a clear benefit to separating state management from components (e.g. due to state complexity) we recommend using [Vuex][vuex-docs] over any other Flux pattern. Otherwise, feel free to manage state within the components.
When there's a clear benefit to separating state management from components (e.g. due to state complexity) we recommend using [Vuex](https://vuex.vuejs.org) over any other Flux pattern. Otherwise, feel free to manage state within the components.
Vuex should be strongly considered when:
...
...
@@ -9,7 +9,7 @@ Vuex should be strongly considered when:
- There are complex interactions with Backend, e.g. multiple API calls
- The app involves interacting with backend via both traditional REST API and GraphQL (especially when moving the REST API over to GraphQL is a pending backend task)
_Note:_ All of the below is explained in more detail in the official [Vuex documentation][vuex-docs].
_Note:_ All of the below is explained in more detail in the official [Vuex documentation](https://vuex.vuejs.org).
## Separation of concerns
...
...
@@ -477,8 +477,6 @@ To prevent this error from happening, you need to export an empty function as `d
exportdefault()=>{};
```
[vuex-docs]:https://vuex.vuejs.org
### Two way data binding
When storing form data in Vuex, it is sometimes necessary to update the value stored. The store should never be mutated directly, and an action should be used instead.
@@ -6,13 +6,13 @@ Using semantic HTML plays a key role when it comes to accessibility.
WAI-ARIA (the Accessible Rich Internet Applications specification) defines a way to make Web content and Web applications more accessible to people with disabilities.
> Note: It is [recommended][using-aria] to use semantic elements as the primary method to achieve accessibility rather than adding aria attributes. Adding aria attributes should be seen as a secondary method for creating accessible elements.
> Note: It is [recommended](https://www.w3.org/TR/using-aria/#notes2) to use semantic elements as the primary method to achieve accessibility rather than adding aria attributes. Adding aria attributes should be seen as a secondary method for creating accessible elements.
### Role
The `role` attribute describes the role the element plays in the context of the document.
Check the list of WAI-ARIA roles [here][roles]
Check the list of WAI-ARIA roles [here](https://www.w3.org/TR/wai-aria-1.1/#landmark_roles)
## Icons
...
...
@@ -36,20 +36,11 @@ In forms we should use the `for` attribute in the label statement:
## Testing
1. On MacOS you can use [VoiceOver][voice-over] by pressing `cmd+F5`.
1. On Windows you can use [Narrator][narrator] by pressing Windows logo key + Ctrl + Enter.
1. On MacOS you can use [VoiceOver](https://www.apple.com/accessibility/mac/vision/) by pressing `cmd+F5`.
1. On Windows you can use [Narrator](https://www.microsoft.com/en-us/accessibility/windows) by pressing Windows logo key + Ctrl + Enter.
## Online resources
-[Chrome Accessibility Developer Tools][dev-tools] for testing accessibility
-[Audit Rules Page][audit-rules] for best practices
-[Lighthouse Accessibility Score][lighthouse] for accessibility audits
In December 2018, Tiago Botelho hosted a [Deep Dive] on GitLab's [Pull Repository Mirroring functionality] to share his domain specific knowledge with anyone who may work in this part of the code base in the future. You can find the [recording on YouTube], and the slides in [PDF]. Everything covered in this deep dive was accurate as of GitLab 11.6, and while specific details may have changed since then, it should still serve as a good introduction.
In December 2018, Tiago Botelho hosted [a Deep Dive](`https://gitlab.com/gitlab-org/create-stage/issues/1`)
on GitLab's [Pull Repository Mirroring functionality](../user/project/repository/repository_mirroring.md#pulling-from-a-remote-repository-starter)
to share his domain specific knowledge with anyone who may work in this part of the
code base in the future. You can find the [recording on YouTube](https://www.youtube.com/watch?v=sSZq0fpdY-Y),
and the slides in [PDF](https://gitlab.com/gitlab-org/create-stage/uploads/8693404888a941fd851f8a8ecdec9675/Gitlab_Create_-_Pull_Mirroring_Deep_Dive.pdf).
Everything covered in this deep dive was accurate as of GitLab 11.6, and while specific
details may have changed since then, it should still serve as a good introduction.
The impact of an SSRF can vary, depending on what the application server
...
...
@@ -155,7 +153,7 @@ The preferred SSRF mitigations within GitLab are:
#### GitLab HTTP Library
The [GitLab::HTTP][2] wrapper library has grown to include mitigations for all of the GitLab-known SSRF vectors. It is also configured to respect the
The [GitLab::HTTP](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/http.rb) wrapper library has grown to include mitigations for all of the GitLab-known SSRF vectors. It is also configured to respect the
`Outbound requests` options that allow instance administrators to block all internal connections, or limit the networks to which connections can be made.
In some cases, it has been possible to configure GitLab::HTTP as the HTTP
...
...
@@ -164,8 +162,6 @@ the mitigations for a new feature.
For situtions in which a whitelist or GitLab:HTTP cannot be used, it will be necessary to implement mitigations directly in the feature. It is best to validate the destination IP addresses themselves, not just domain names, as DNS can be controlled by the attacker. Below are a list of mitigations that should be implemented.
...
...
@@ -185,9 +181,7 @@ For situtions in which a whitelist or GitLab:HTTP cannot be used, it will be nec
- For HTTP connections: Disable redirects or validate the redirect destination
- To mitigate DNS rebinding attacks, validate and use the first IP address received
See [url_blocker_spec.rb][3] for examples of SSRF payloads
and use methods which are deprecated in Rails 5 ([#23768](https://gitlab.com/gitlab-org/gitlab/-/issues/16260)).
### About Karma
...
...
@@ -356,7 +354,7 @@ possible).
| Tests path | Testing engine | Notes |
| ---------- | -------------- | ----- |
| `spec/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) | If your test has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. |
| `spec/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) | If your test has the `:js` metadata, the browser driver will be [Poltergeist](https://github.com/teamcapybara/capybara#poltergeist), otherwise it's using [RackTest](https://github.com/teamcapybara/capybara#racktest). |
### Frontend feature tests
...
...
@@ -460,9 +458,6 @@ The reasons why we should follow these best practices are as follows:
of tests). This is slower than transactions, however, so we want to use
GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse],
[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces
are configured and packaged by [GitLab Omnibus].
GitLab consists of [multiple pieces](../architecture.md#components) such as [GitLab Shell](https://gitlab.com/gitlab-org/gitlab-shell), [GitLab Workhorse](https://gitlab.com/gitlab-org/gitlab-workhorse),
[Gitaly](https://gitlab.com/gitlab-org/gitaly), [GitLab Pages](https://gitlab.com/gitlab-org/gitlab-pages), [GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner), and GitLab Rails. All theses pieces
are configured and packaged by [GitLab Omnibus](https://gitlab.com/gitlab-org/omnibus-gitlab).
The QA framework and instance-level scenarios are [part of GitLab Rails] so that
The QA framework and instance-level scenarios are [part of GitLab Rails](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/qa) so that
they're always in-sync with the codebase (especially the views).
Note that:
...
...
@@ -483,11 +478,11 @@ Note that:
- data needed for the tests can only be created using the GUI or the API
- expectations can only be made against the browser page and API responses
Every new feature should come with a [test plan].
Every new feature should come with a [test plan](https://gitlab.com/gitlab-org/gitlab/tree/master/.gitlab/issue_templates/Test%20plan.md).
| Tests path | Testing engine | Notes |
| ---------- | -------------- | ----- |
| `qa/qa/specs/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) + Custom QA framework | Tests should be placed under their corresponding [Product category] |
| `qa/qa/specs/features/` | [Capybara](https://github.com/teamcapybara/capybara) + [RSpec](https://github.com/rspec/rspec-rails#feature-specs) + Custom QA framework | Tests should be placed under their corresponding [Product category](https://about.gitlab.com/handbook/product/categories/) |
> See [end-to-end tests](end_to_end/index.md) for more information.
...
...
@@ -495,17 +490,6 @@ Note that `qa/spec` contains unit tests of the QA framework itself, not to be
confused with the application's [unit tests](#unit-tests) or
