summaryrefslogtreecommitdiff
path: root/doc/development
diff options
context:
space:
mode:
authorPhil Hughes <me@iamphill.com>2016-08-30 10:43:09 +0100
committerPhil Hughes <me@iamphill.com>2016-08-30 10:43:09 +0100
commit85f6244ce852fb6b788ea660c7d0cbe14ec10a20 (patch)
treeec29f6c01ea8e4ac774f3dae874a3a3abf97267b /doc/development
parent2bee8e7db927d2bc2c5912b98dfe52d3c3c40fbd (diff)
parent2778dec131c2afac9fcdb2c42365b69099a5ae5b (diff)
downloadgitlab-ce-85f6244ce852fb6b788ea660c7d0cbe14ec10a20.tar.gz
Merge branch 'master' into build-cancel-spinner
Diffstat (limited to 'doc/development')
-rw-r--r--doc/development/README.md37
-rw-r--r--doc/development/adding_database_indexes.md123
-rw-r--r--doc/development/doc_styleguide.md121
-rw-r--r--doc/development/gotchas.md5
-rw-r--r--doc/development/migration_style_guide.md46
-rw-r--r--doc/development/newlines_styleguide.md102
-rw-r--r--doc/development/performance.md9
-rw-r--r--doc/development/rake_tasks.md35
-rw-r--r--doc/development/ui_guide.md92
-rw-r--r--doc/development/what_requires_downtime.md161
10 files changed, 643 insertions, 88 deletions
diff --git a/doc/development/README.md b/doc/development/README.md
index c5d5af43864..57f37da6f80 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -1,18 +1,41 @@
# Development
+## Outside of docs
+
+- [CONTRIBUTING.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md) main contributing guide
+- [PROCESS.md](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md) contributing process
+- [GitLab Development Kit (GDK)](https://gitlab.com/gitlab-org/gitlab-development-kit/blob/master/doc/howto/README.md) to install a development version
+
+## Styleguides
+
+- [Documentation styleguide](doc_styleguide.md) Use this styleguide if you are
+ contributing to documentation.
+- [SQL Migration Style Guide](migration_style_guide.md) for creating safe SQL migrations
+- [Testing standards and style guidelines](testing.md)
+- [UI guide](ui_guide.md) for building GitLab with existing CSS styles and elements
+- [SQL guidelines](sql.md) for SQL guidelines
+
+## Process
+
+- [Code review guidelines](code_review.md) for reviewing code and having code reviewed.
+
+## Backend howtos
+
- [Architecture](architecture.md) of GitLab
- [CI setup](ci_setup.md) for testing GitLab
-- [Code review guidelines](code_review.md) for reviewing code and having code
- reviewed.
- [Gotchas](gotchas.md) to avoid
- [How to dump production data to staging](db_dump.md)
- [Instrumentation](instrumentation.md)
-- [Licensing](licensing.md) for ensuring license compliance
-- [Migration Style Guide](migration_style_guide.md) for creating safe migrations
- [Performance guidelines](performance.md)
- [Rake tasks](rake_tasks.md) for development
- [Shell commands](shell_commands.md) in the GitLab codebase
- [Sidekiq debugging](sidekiq_debugging.md)
-- [SQL guidelines](sql.md) for SQL guidelines
-- [Testing standards and style guidelines](testing.md)
-- [UI guide](ui_guide.md) for building GitLab with existing css styles and elements
+
+## Databases
+
+- [What requires downtime?](what_requires_downtime.md)
+- [Adding database indexes](adding_database_indexes.md)
+
+## Compliance
+
+- [Licensing](licensing.md) for ensuring license compliance
diff --git a/doc/development/adding_database_indexes.md b/doc/development/adding_database_indexes.md
new file mode 100644
index 00000000000..ea6f14da3b9
--- /dev/null
+++ b/doc/development/adding_database_indexes.md
@@ -0,0 +1,123 @@
+# Adding Database Indexes
+
+Indexes can be used to speed up database queries, but when should you add a new
+index? Traditionally the answer to this question has been to add an index for
+every column used for filtering or joining data. For example, consider the
+following query:
+
+```sql
+SELECT *
+FROM projects
+WHERE user_id = 2;
+```
+
+Here we are filtering by the `user_id` column and as such a developer may decide
+to index this column.
+
+While in certain cases indexing columns using the above approach may make sense
+it can actually have a negative impact. Whenever you write data to a table any
+existing indexes need to be updated. The more indexes there are the slower this
+can potentially become. Indexes can also take up quite some disk space depending
+on the amount of data indexed and the index type. For example, PostgreSQL offers
+"GIN" indexes which can be used to index certain data types that can not be
+indexed by regular btree indexes. These indexes however generally take up more
+data and are slower to update compared to btree indexes.
+
+Because of all this one should not blindly add a new index for every column used
+to filter data by. Instead one should ask themselves the following questions:
+
+1. Can I write my query in such a way that it re-uses as many existing indexes
+ as possible?
+2. Is the data going to be large enough that using an index will actually be
+ faster than just iterating over the rows in the table?
+3. Is the overhead of maintaining the index worth the reduction in query
+ timings?
+
+We'll explore every question in detail below.
+
+## Re-using Queries
+
+The first step is to make sure your query re-uses as many existing indexes as
+possible. For example, consider the following query:
+
+```sql
+SELECT *
+FROM todos
+WHERE user_id = 123
+AND state = 'open';
+```
+
+Now imagine we already have an index on the `user_id` column but not on the
+`state` column. One may think this query will perform badly due to `state` being
+unindexed. In reality the query may perform just fine given the index on
+`user_id` can filter out enough rows.
+
+The best way to determine if indexes are re-used is to run your query using
+`EXPLAIN ANALYZE`. Depending on any extra tables that may be joined and
+other columns being used for filtering you may find an extra index is not going
+to make much (if any) difference. On the other hand you may determine that the
+index _may_ make a difference.
+
+In short:
+
+1. Try to write your query in such a way that it re-uses as many existing
+ indexes as possible.
+2. Run the query using `EXPLAIN ANALYZE` and study the output to find the most
+ ideal query.
+
+## Data Size
+
+A database may decide not to use an index despite it existing in case a regular
+sequence scan (= simply iterating over all existing rows) is faster. This is
+especially the case for small tables.
+
+If a table is expected to grow in size and you expect your query has to filter
+out a lot of rows you may want to consider adding an index. If the table size is
+very small (e.g. only a handful of rows) or any existing indexes filter out
+enough rows you may _not_ want to add a new index.
+
+## Maintenance Overhead
+
+Indexes have to be updated on every table write. In case of PostgreSQL _all_
+existing indexes will be updated whenever data is written to a table. As a
+result of this having many indexes on the same table will slow down writes.
+
+Because of this one should ask themselves: is the reduction in query performance
+worth the overhead of maintaining an extra index?
+
+If adding an index reduces SELECT timings by 5 milliseconds but increases
+INSERT/UPDATE/DELETE timings by 10 milliseconds then the index may not be worth
+it. On the other hand, if SELECT timings are reduced but INSERT/UPDATE/DELETE
+timings are not affected you may want to add the index after all.
+
+## Finding Unused Indexes
+
+To see which indexes are unused you can run the following query:
+
+```sql
+SELECT relname as table_name, indexrelname as index_name, idx_scan, idx_tup_read, idx_tup_fetch, pg_size_pretty(pg_relation_size(indexrelname::regclass))
+FROM pg_stat_all_indexes
+WHERE schemaname = 'public'
+AND idx_scan = 0
+AND idx_tup_read = 0
+AND idx_tup_fetch = 0
+ORDER BY pg_relation_size(indexrelname::regclass) desc;
+```
+
+This query outputs a list containing all indexes that are never used and sorts
+them by indexes sizes in descending order. This query can be useful to
+determine if any previously indexes are useful after all. More information on
+the meaning of the various columns can be found at
+<https://www.postgresql.org/docs/current/static/monitoring-stats.html>.
+
+Because the output of this query relies on the actual usage of your database it
+may be affected by factors such as (but not limited to):
+
+* Certain queries never being executed, thus not being able to use certain
+ indexes.
+* Certain tables having little data, resulting in PostgreSQL using sequence
+ scans instead of index scans.
+
+In other words, this data is only reliable for a frequently used database with
+plenty of data and with as many GitLab features enabled (and being used) as
+possible.
diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md
index 975bb82c37d..37bb59e112c 100644
--- a/doc/development/doc_styleguide.md
+++ b/doc/development/doc_styleguide.md
@@ -3,12 +3,64 @@
This styleguide recommends best practices to improve documentation and to keep
it organized and easy to find.
-## Naming
+## Location and naming of documents
-- When creating a new document and it has more than one word in its name,
- make sure to use underscores instead of spaces or dashes (`-`). For example,
- a proper naming would be `import_projects_from_github.md`. The same rule
- applies to images.
+>**Note:**
+These guidelines derive from the discussion taken place in issue [#3349][ce-3349].
+
+The documentation hierarchy can be vastly improved by providing a better layout
+and organization of directories.
+
+Having a structured document layout, we will be able to have meaningful URLs
+like `docs.gitlab.com/user/project/merge_requests.html`. With this pattern,
+you can immediately tell that you are navigating a user related documentation
+and is about the project and its merge requests.
+
+The table below shows what kind of documentation goes where.
+
+| Directory | What belongs here |
+| --------- | -------------- |
+| `doc/user/` | User related documentation. Anything that can be done within the GitLab UI goes here including `/admin`. |
+| `doc/administration/` | Documentation that requires the user to have access to the server where GitLab is installed. The admin settings that can be accessed via GitLab's interface go under `doc/user/admin_area/`. |
+| `doc/api/` | API related documentation. |
+| `doc/development/` | Documentation related to the development of GitLab. Any styleguides should go here. |
+| `doc/legal/` | Legal documents about contributing to GitLab. |
+| `doc/install/`| Probably the most visited directory, since `installation.md` is there. Ideally this should go under `doc/administration/`, but it's best to leave it as-is in order to avoid confusion (still debated though). |
+| `doc/update/` | Same with `doc/install/`. Should be under `administration/`, but this is a well known location, better leave as-is, at least for now. |
+
+---
+
+**General rules:**
+
+1. The correct naming and location of a new document, is a combination
+ of the relative URL of the document in question and the GitLab Map design
+ that is used for UX purposes ([source][graffle], [image][gitlab-map]).
+1. When creating a new document and it has more than one word in its name,
+ make sure to use underscores instead of spaces or dashes (`-`). For example,
+ a proper naming would be `import_projects_from_github.md`. The same rule
+ applies to images.
+1. There are four main directories, `user`, `administration`, `api` and `development`.
+1. The `doc/user/` directory has five main subdirectories: `project/`, `group/`,
+ `profile/`, `dashboard/` and `admin_area/`.
+ 1. `doc/user/project/` should contain all project related documentation.
+ 1. `doc/user/group/` should contain all group related documentation.
+ 1. `doc/user/profile/` should contain all profile related documentation.
+ Every page you would navigate under `/profile` should have its own document,
+ i.e. `account.md`, `applications.md`, `emails.md`, etc.
+ 1. `doc/user/dashboard/` should contain all dashboard related documentation.
+ 1. `doc/user/admin_area/` should contain all admin related documentation
+ describing what can be achieved by accessing GitLab's admin interface
+ (_not to be confused with `doc/administration` where server access is
+ required_).
+ 1. Every category under `/admin/application_settings` should have its
+ own document located at `doc/user/admin_area/settings/`. For example,
+ the **Visibility and Access Controls** category should have a document
+ located at `doc/user/admin_area/settings/visibility_and_access_controls.md`.
+
+---
+
+If you are unsure where a document should live, you can ping `@axil` in your
+merge request.
## Text
@@ -44,7 +96,7 @@ it organized and easy to find.
- When introducing a new document, be careful for the headings to be
grammatically and syntactically correct. It is advised to mention one or all
of the following GitLab members for a review: `@axil`, `@rspeicher`,
- `@dblessing`, `@ashleys`, `@nearlythere`. This is to ensure that no document
+ `@dblessing`, `@ashleys`. 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 newline after a heading
@@ -103,15 +155,15 @@ Inside the document:
- Every piece of documentation that comes with a new feature should declare the
GitLab version that feature got introduced. Right below the heading add a
- note: `>**Note:** This feature was introduced in GitLab 8.3`
+ note: `> Introduced in GitLab 8.3.`.
- If possible every feature should have a link to the MR that introduced it.
The above note would be then transformed to:
- `>**Note:** This feature was [introduced][ce-1242] in GitLab 8.3`, where
+ `> [Introduced][ce-1242] in GitLab 8.3.`, where
the [link identifier](#links) is named after the repository (CE) and the MR
- number
+ number.
- If the feature is only in GitLab EE, don't forget to mention it, like:
- `>**Note:** This feature was introduced in GitLab EE 8.3`. Otherwise, leave
- this mention out
+ `> Introduced in GitLab EE 8.3.`. Otherwise, leave
+ this mention out.
## References
@@ -170,18 +222,26 @@ For example, if you were to move `doc/workflow/lfs/lfs_administration.md` to
```
1. Find and replace any occurrences of the old location with the new one.
- A quick way to find them is to use `grep`:
+ A quick way to find them is to use `git grep`. First go to the root directory
+ where you cloned the `gitlab-ce` repository and then do:
```
- grep -nR "lfs_administration.md" doc/
+ git grep -n "workflow/lfs/lfs_administration"
+ git grep -n "lfs/lfs_administration"
```
- The above command will search in the `doc/` directory for
- `lfs_administration.md` recursively and will print the file and the line
- where this file is mentioned. Note that we used just the filename
- (`lfs_administration.md`) and not the whole the relative path
- (`workflow/lfs/lfs_administration.md`).
+Things to note:
+- Since we also use inline documentation, except for the documentation itself,
+ the document might also be referenced in the views of GitLab (`app/`) which will
+ render when visiting `/help`, and sometimes in the testing suite (`spec/`).
+- The above `git grep` command will search recursively in the directory you run
+ it in for `workflow/lfs/lfs_administration` and `lfs/lfs_administration`
+ and will print the file and the line where this file is mentioned.
+ You may ask why the two greps. Since we use relative paths to link to
+ documentation, sometimes it might be useful to search a path deeper.
+- The `*.md` extension is not used when a document is linked to GitLab's
+ built-in help page, that's why we omit it in `git grep`.
## Configuration documentation for source and Omnibus installations
@@ -244,6 +304,12 @@ In this case:
Here is a list of must-have items. Use them in the exact order that appears
on this document. Further explanation is given below.
+- Every method must be described using [Grape's DSL](https://github.com/ruby-grape/grape/tree/v0.13.0#describing-methods)
+ (see https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/api/environments.rb
+ for a good example):
+ - `desc` for the method summary (you can pass it a block for additional details)
+ - `params` for the method params (this acts as description **and** validation
+ of the params)
- Every method must have the REST API request. For example:
```
@@ -297,7 +363,7 @@ Below is a set of [cURL][] examples that you can use in the API documentation.
Get the details of a group:
```bash
-curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/gitlab-org
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/groups/gitlab-org
```
#### cURL example with parameters passed in the URL
@@ -305,7 +371,7 @@ curl -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/
Create a new project under the authenticated user's namespace:
```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects?name=foo"
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects?name=foo"
```
#### Post data using cURL's --data
@@ -315,7 +381,7 @@ cURL's `--data` option. The example below will create a new project `foo` under
the authenticated user's namespace.
```bash
-curl --data "name=foo" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects"
+curl --data "name=foo" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects"
```
#### Post data using JSON content
@@ -324,7 +390,7 @@ curl --data "name=foo" -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.
and double quotes.
```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -H "Content-Type: application/json" --data '{"path": "my-group", "name": "My group"}' https://gitlab.example.com/api/v3/groups
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --header "Content-Type: application/json" --data '{"path": "my-group", "name": "My group"}' https://gitlab.example.com/api/v3/groups
```
#### Post data using form-data
@@ -333,7 +399,7 @@ Instead of using JSON or urlencode you can use multipart/form-data which
properly handles data encoding:
```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -F "title=ssh-key" -F "key=ssh-rsa AAAAB3NzaC1yc2EA..." https://gitlab.example.com/api/v3/users/25/keys
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form "title=ssh-key" --form "key=ssh-rsa AAAAB3NzaC1yc2EA..." https://gitlab.example.com/api/v3/users/25/keys
```
The above example is run by and administrator and will add an SSH public key
@@ -347,7 +413,7 @@ contains spaces in its title. Observe how spaces are escaped using the `%20`
ASCII code.
```bash
-curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/42/issues?title=Hello%20Dude"
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/42/issues?title=Hello%20Dude"
```
Use `%2F` for slashes (`/`).
@@ -359,10 +425,13 @@ restrict the sign-up e-mail domains of a GitLab instance to `*.example.com` and
`example.net`, you would do something like this:
```bash
-curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -d "restricted_signup_domains[]=*.example.com" -d "restricted_signup_domains[]=example.net" https://gitlab.example.com/api/v3/application/settings
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --data "domain_whitelist[]=*.example.com" --data "domain_whitelist[]=example.net" https://gitlab.example.com/api/v3/application/settings
```
[cURL]: http://curl.haxx.se/ "cURL website"
[single spaces]: http://www.slate.com/articles/technology/technology/2011/01/space_invaders.html
-[gfm]: http://docs.gitlab.com/ce/markdown/markdown.html#newlines "GitLab flavored markdown documentation"
+[gfm]: http://docs.gitlab.com/ce/user/markdown.html#newlines "GitLab flavored markdown documentation"
[doc-restart]: ../administration/restart_gitlab.md "GitLab restart documentation"
+[ce-3349]: https://gitlab.com/gitlab-org/gitlab-ce/issues/3349 "Documentation restructure"
+[graffle]: https://gitlab.com/gitlab-org/gitlab-design/blob/d8d39f4a87b90fb9ae89ca12dc565347b4900d5e/production/resources/gitlab-map.graffle
+[gitlab-map]: https://gitlab.com/gitlab-org/gitlab-design/raw/master/production/resources/gitlab-map.png
diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md
index 9d7fe7440d2..159d5ce286d 100644
--- a/doc/development/gotchas.md
+++ b/doc/development/gotchas.md
@@ -41,10 +41,10 @@ Rubocop](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/.rubocop.yml#L9
[Exception]: http://stackoverflow.com/q/10048173/223897
-## Don't use inline CoffeeScript in views
+## Don't use inline CoffeeScript/JavaScript in views
Using the inline `:coffee` or `:coffeescript` Haml filters comes with a
-performance overhead.
+performance overhead. Using inline JavaScript is not a good way to structure your code and should be avoided.
_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/config/initializers/hamlit.rb)
in an initializer._
@@ -52,6 +52,7 @@ in an initializer._
### Further reading
- Pull Request: [Replace CoffeeScript block into JavaScript in Views](https://git.io/vztMu)
+- Stack Overflow: [Why you should not write inline JavaScript](http://programmers.stackexchange.com/questions/86589/why-should-i-avoid-inline-scripting)
- Stack Overflow: [Performance implications of using :coffescript filter inside HAML templates?](http://stackoverflow.com/a/17571242/223897)
## ID-based CSS selectors need to be a bit more specific
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index e2ca46504e7..b8fab3aaff7 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -11,7 +11,8 @@ migrations are written carefully, can be applied online and adhere to the style
Migrations should not require GitLab installations to be taken offline unless
_absolutely_ necessary. If a migration requires downtime this should be
clearly mentioned during the review process as well as being documented in the
-monthly release post.
+monthly release post. For more information see the "Downtime Tagging" section
+below.
When writing your migrations, also consider that databases might have stale data
or inconsistencies and guard for that. Try to make as little assumptions as possible
@@ -20,35 +21,34 @@ about the state of the database.
Please don't depend on GitLab specific code since it can change in future versions.
If needed copy-paste GitLab code into the migration to make it forward compatible.
-## Comments in the migration
+## Downtime Tagging
-Each migration you write needs to have the two following pieces of information
-as comments.
+Every migration must specify if it requires downtime or not, and if it should
+require downtime it must also specify a reason for this. To do so, add the
+following two constants to the migration class' body:
-### Online, Offline, errors?
+* `DOWNTIME`: a boolean that when set to `true` indicates the migration requires
+ downtime.
+* `DOWNTIME_REASON`: a String containing the reason for the migration requiring
+ downtime. This constant **must** be set when `DOWNTIME` is set to `true`.
-First, you need to provide information on whether the migration can be applied:
+For example:
-1. online without errors (works on previous version and new one)
-2. online with errors on old instances after migrating
-3. online with errors on new instances while migrating
-4. offline (needs to happen without app servers to prevent db corruption)
-
-For example:
-
-```
-# Migration type: online without errors (works on previous version and new one)
+```ruby
class MyMigration < ActiveRecord::Migration
-...
-```
+ DOWNTIME = true
+ DOWNTIME_REASON = 'This migration requires downtime because ...'
-It is always preferable to have a migration run online. If you expect the migration
-to take particularly long (for instance, if it loops through all notes),
-this is valuable information to add.
+ def change
+ ...
+ end
+end
+```
-If you don't provide the information it means that a migration is safe to run online.
+It is an error (that is, CI will fail) if the `DOWNTIME` constant is missing
+from a migration class.
-### Reversibility
+## Reversibility
Your migration should be reversible. This is very important, as it should
be possible to downgrade in case of a vulnerability or bugs.
@@ -100,7 +100,7 @@ value of `10` you'd write the following:
class MyMigration < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
-
+
def up
add_column_with_default(:projects, :foo, :integer, default: 10)
end
diff --git a/doc/development/newlines_styleguide.md b/doc/development/newlines_styleguide.md
new file mode 100644
index 00000000000..e03adcaadea
--- /dev/null
+++ b/doc/development/newlines_styleguide.md
@@ -0,0 +1,102 @@
+# Newlines styleguide
+
+This style guide recommends best practices for newlines in Ruby code.
+
+## Rule: separate code with newlines only when it makes sense from logic perspectice
+
+```ruby
+# bad
+def method
+ issue = Issue.new
+
+ issue.save
+
+ render json: issue
+end
+```
+
+```ruby
+# good
+def method
+ issue = Issue.new
+ issue.save
+
+ render json: issue
+end
+```
+
+## Rule: separate code and block with newlines
+
+### Newline before block
+
+```ruby
+# bad
+def method
+ issue = Issue.new
+ if issue.save
+ render json: issue
+ end
+end
+```
+
+```ruby
+# good
+def method
+ issue = Issue.new
+
+ if issue.save
+ render json: issue
+ end
+end
+```
+
+## Newline after block
+
+```ruby
+# bad
+def method
+ if issue.save
+ issue.send_email
+ end
+ render json: issue
+end
+```
+
+```ruby
+# good
+def method
+ if issue.save
+ issue.send_email
+ end
+
+ render json: issue
+end
+```
+
+### Exception: no need for newline when code block starts or ends right inside another code block
+
+```ruby
+# bad
+def method
+
+ if issue
+
+ if issue.valid?
+ issue.save
+ end
+
+ end
+
+end
+```
+
+```ruby
+# good
+def method
+ if issue
+ if issue.valid?
+ issue.save
+ end
+ end
+end
+```
diff --git a/doc/development/performance.md b/doc/development/performance.md
index fb37b3a889c..7ff603e2c4a 100644
--- a/doc/development/performance.md
+++ b/doc/development/performance.md
@@ -15,8 +15,8 @@ The process of solving performance problems is roughly as follows:
3. Add your findings based on the measurement period (screenshots of graphs,
timings, etc) to the issue mentioned in step 1.
4. Solve the problem.
-5. Create a merge request, assign the "performance" label and ping the right
- people (e.g. [@yorickpeterse][yorickpeterse] and [@joshfng][joshfng]).
+5. Create a merge request, assign the "Performance" label and assign it to
+ [@yorickpeterse][yorickpeterse] for reviewing.
6. Once a change has been deployed make sure to _again_ measure for at least 24
hours to see if your changes have any impact on the production environment.
7. Repeat until you're done.
@@ -36,8 +36,8 @@ graphs/dashboards.
GitLab provides two built-in tools to aid the process of improving performance:
-* [Sherlock](doc/development/profiling.md#sherlock)
-* [GitLab Performance Monitoring](doc/monitoring/performance/monitoring.md)
+* [Sherlock](profiling.md#sherlock)
+* [GitLab Performance Monitoring](../monitoring/performance/monitoring.md)
GitLab employees can use GitLab.com's performance monitoring systems located at
<http://performance.gitlab.net>, this requires you to log in using your
@@ -254,5 +254,4 @@ referencing an object directly may even slow code down.
[#15607]: https://gitlab.com/gitlab-org/gitlab-ce/issues/15607
[yorickpeterse]: https://gitlab.com/u/yorickpeterse
-[joshfng]: https://gitlab.com/u/joshfng
[anti-pattern]: https://en.wikipedia.org/wiki/Anti-pattern
diff --git a/doc/development/rake_tasks.md b/doc/development/rake_tasks.md
index 41685c7ee41..a7175f3f87e 100644
--- a/doc/development/rake_tasks.md
+++ b/doc/development/rake_tasks.md
@@ -14,11 +14,33 @@ Note: `db:setup` calls `db:seed` but this does nothing.
## Run tests
-This runs all test suites present in GitLab.
+In order to run the test you can use the following commands:
+- `rake spinach` to run the spinach suite
+- `rake spec` to run the rspec suite
+- `rake teaspoon` to run the teaspoon test suite
+- `rake gitlab:test` to run all the tests
-```
-bundle exec rake test
-```
+Note: Both `rake spinach` and `rake spec` takes significant time to pass.
+Instead of running full test suite locally you can save a lot of time by running
+a single test or directory related to your changes. After you submit merge request
+CI will run full test suite for you. Green CI status in the merge request means
+full test suite is passed.
+
+Note: You can't run `rspec .` since this will try to run all the `_spec.rb`
+files it can find, also the ones in `/tmp`
+
+To run a single test file you can use:
+
+- `bundle exec rspec spec/controllers/commit_controller_spec.rb` for a rspec test
+- `bundle exec spinach features/project/issues/milestones.feature` for a spinach test
+
+To run several tests inside one directory:
+
+- `bundle exec rspec spec/requests/api/` for the rspec tests if you want to test API only
+- `bundle exec spinach features/profile/` for the spinach tests if you want to test only profile pages
+
+If you want to use [Spring](https://github.com/rails/spring) set
+`ENABLE_SPRING=1` in your environment.
## Generate searchable docs for source code
@@ -53,3 +75,8 @@ Generating a sprite file containing all the Emoji can be done by running:
```
bundle exec rake gemojione:sprite
```
+
+If new emoji are added, the spritesheet may change size. To compensate for
+such changes, first generate the `emoji.png` spritesheet with the above Rake
+task, then check the dimensions of the new spritesheet and update the
+`SPRITESHEET_WIDTH` and `SPRITESHEET_HEIGHT` constants accordingly.
diff --git a/doc/development/ui_guide.md b/doc/development/ui_guide.md
index ce0aaa2fd25..2d1d504202c 100644
--- a/doc/development/ui_guide.md
+++ b/doc/development/ui_guide.md
@@ -1,43 +1,48 @@
-# UI Guide for building GitLab
+# UI Guide for building GitLab
## GitLab UI development kit
We created a page inside GitLab where you can check commonly used html and css elements.
-When you run GitLab instance locally - just visit http://localhost:3000/help/ui page to see UI examples
+When you run GitLab instance locally - just visit http://localhost:3000/help/ui page to see UI examples
you can use during GitLab development.
## Design repository
-All design files are stored in the [gitlab-design](https://gitlab.com/gitlab-org/gitlab-design)
-repository and maintained by GitLab UX designers.
+All design files are stored in the [gitlab-design](https://gitlab.com/gitlab-org/gitlab-design)
+repository and maintained by GitLab UX designers.
## Navigation
-GitLab's layout contains 2 sections: the left sidebar and the content. The left sidebar contains a static navigation menu.
-This menu will be visible regardless of what page you visit. The left sidebar also contains the GitLab logo
-and the current user's profile picture. The content section contains a header and the content itself.
-The header describes the current GitLab page and what navigation is
-available to user in this area. Depending on the area (project, group, profile setting) the header name and navigation may change. For example when user visits one of the
-project pages the header will contain a project name and navigation for that project. When the user visits a group page it will contain a group name and navigation related to this group.
+GitLab's layout contains 2 sections: the left sidebar and the content. The left sidebar contains a static navigation menu.
+This menu will be visible regardless of what page you visit.
+The content section contains a header and the content itself. The header describes the current GitLab page and what navigation is
+available to the user in this area. Depending on the area (project, group, profile setting) the header name and navigation may change. For example, when the user visits one of the
+project pages the header will contain the project's name and navigation for that project. When the user visits a group page it will contain the group's name and navigation related to this group.
+
+You can see a visual representation of the navigation in GitLab in the GitLab Product Map, which is located in the [Design Repository](gitlab-map-graffle)
+along with [PDF](gitlab-map-pdf) and [PNG](gitlab-map-png) exports.
+
### Adding new tab to header navigation
-We try to keep the amount of tabs in the header navigation between 5 and 10 so that it fits on a typical laptop screen. We also try not to confuse the user with too many options. Ideally each
-tab should represent separate functionality. Everything related to the issue
-tracker should be under the 'Issues' tab while everything related to the wiki should
+We try to keep the amount of tabs in the header navigation between 5 and 10 so that it fits on a typical laptop screen. We also try not to confuse the user with too many options. Ideally each
+tab should represent separate functionality. Everything related to the issue
+tracker should be under the 'Issues' tab while everything related to the wiki should
be under 'Wiki' tab and so on and so forth.
+When adding a new tab to the header don't use more than 2 words for text in the link.
+We want to keep links short and easy to remember and fit all of them in the small screen.
-## Mobile screen size
+## Mobile screen size
-We want GitLab to work well on small mobile screens as well. Size limitations make it is impossible to fit everything on a mobile screen. In this case it is OK to hide
-part of the UI for smaller resolutions in favor of a better user experience.
+We want GitLab to work well on small mobile screens as well. Size limitations make it is impossible to fit everything on a mobile screen. In this case it is OK to hide
+part of the UI for smaller resolutions in favor of a better user experience.
However core functionality like browsing files, creating issues, writing comments, should
be available on all resolutions.
## Icons
-* `trash` icon for button or link that does destructive action like removing
+* `trash` icon for button or link that does destructive action like removing
information from database or file system
* `x` icon for closing/hiding UI element. For example close modal window
* `pencil` icon for edit button or link
@@ -45,13 +50,58 @@ information from database or file system
* `rss` for rss/atom feed
* `plus` for link or dropdown that lead to page where you create new object (For example new issue page)
+### SVGs
+
+When exporting SVGs, be sure to follow the following guidelines:
+
+1. Convert all strokes to outlines.
+2. Use pathfinder tools to combine overlapping paths and create compound paths.
+3. SVGs that are limited to one color should be exported without a fill color so the color can be set using CSS.
+4. Ensure that exported SVGs have been run through an [SVG cleaner](https://github.com/RazrFalcon/SVGCleaner) to remove unused elements and attributes.
+
+You can open your svg in a text editor to ensure that it is clean.
+Incorrect files will look like this:
+
+```xml
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg width="16px" height="17px" viewBox="0 0 16 17" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+ <!-- Generator: Sketch 3.7.2 (28276) - http://www.bohemiancoding.com/sketch -->
+ <title>Group</title>
+ <desc>Created with Sketch.</desc>
+ <defs></defs>
+ <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
+ <g id="Group" fill="#7E7C7C">
+ <path d="M15.1111,1 L0.8891,1 C0.3981,1 0.0001,1.446 0.0001,1.996 L0.0001,15.945 C0.0001,16.495 0.3981,16.941 0.8891,16.941 L15.1111,16.941 C15.6021,16.941 16.0001,16.495 16.0001,15.945 L16.0001,1.996 C16.0001,1.446 15.6021,1 15.1111,1 L15.1111,1 L15.1111,1 Z M14.0001,6.0002 L14.0001,14.949 L2.0001,14.949 L2.0001,6.0002 L14.0001,6.0002 Z M14.0001,4.0002 L14.0001,2.993 L2.0001,2.993 L2.0001,4.0002 L14.0001,4.0002 Z" id="Combined-Shape"></path>
+ <polygon id="Fill-11" points="3 2.0002 5 2.0002 5 0.0002 3 0.0002"></polygon>
+ <polygon id="Fill-16" points="11 2.0002 13 2.0002 13 0.0002 11 0.0002"></polygon>
+ <path d="M5.37709616,11.5511984 L6.92309616,12.7821984 C7.35112915,13.123019 7.97359761,13.0565604 8.32002627,12.6330535 L10.7740263,9.63305349 C11.1237073,9.20557058 11.0606364,8.57555475 10.6331535,8.22587373 C10.2056706,7.87619272 9.57565475,7.93926361 9.22597373,8.36674651 L6.77197373,11.3667465 L8.16890384,11.2176016 L6.62290384,9.98660159 C6.19085236,9.6425813 5.56172188,9.71394467 5.21770159,10.1459962 C4.8736813,10.5780476 4.94504467,11.2071781 5.37709616,11.5511984 L5.37709616,11.5511984 Z" id="Stroke-21"></path>
+ </g>
+ </g>
+</svg>
+```
+
+Correct file will look like this:
+
+```xml
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 17" enable-background="new 0 0 16 17"><path d="m15.1 1h-2.1v-1h-2v1h-6v-1h-2v1h-2.1c-.5 0-.9.5-.9 1v14c0 .6.4 1 .9 1h14.2c.5 0 .9-.4.9-1v-14c0-.5-.4-1-.9-1m-1.1 14h-12v-9h12v9m0-11h-12v-1h12v1"/><path d="m5.4 11.6l1.5 1.2c.4.3 1.1.3 1.4-.1l2.5-3c.3-.4.3-1.1-.1-1.4-.5-.4-1.1-.3-1.5.1l-1.8 2.2-.8-.6c-.4-.3-1.1-.3-1.4.2-.3.4-.3 1 .2 1.4"/></svg>
+```
+
## Buttons
* Button should contain icon or text. Exceptions should be approved by UX designer.
* Use red button for destructive actions (not revertable). For example removing issue.
-* Use green or blue button for primary action. Primary button should be only one.
-Do not use both green and blue button in one form.
-* For all other cases use default white button.
-* Text button should have only first word capitalized. So should be "Create issue" instead of "Create Issue"
+* Use green or blue button for primary action. Primary button should be only one.
+Do not use both green and blue button in one form.
+* For all other cases use default white button.
+* Text button should have only first word capitalized. So should be "Create issue" instead of "Create Issue"
+
+## Counts
+
+* Always use the [`number_with_delimiter`][number_with_delimiter] helper to
+ display counts in the UI.
+[number_with_delimiter]: http://api.rubyonrails.org/classes/ActionView/Helpers/NumberHelper.html#method-i-number_with_delimiter
+[gitlab-map-graffle]: https://gitlab.com/gitlab-org/gitlab-design/blob/master/production/resources/gitlab-map.graffle
+[gitlab-map-pdf]: https://gitlab.com/gitlab-org/gitlab-design/raw/master/production/resources/gitlab-map.pdf
+[gitlab-map-png]: https://gitlab.com/gitlab-org/gitlab-design/raw/master/production/resources/gitlab-map.png \ No newline at end of file
diff --git a/doc/development/what_requires_downtime.md b/doc/development/what_requires_downtime.md
new file mode 100644
index 00000000000..2574c2c0472
--- /dev/null
+++ b/doc/development/what_requires_downtime.md
@@ -0,0 +1,161 @@
+# What requires downtime?
+
+When working with a database certain operations can be performed without taking
+GitLab offline, others do require a downtime period. This guide describes
+various operations and their impact.
+
+## Adding Columns
+
+On PostgreSQL you can safely add a new column to an existing table as long as it
+does **not** have a default value. For example, this query would not require
+downtime:
+
+```sql
+ALTER TABLE projects ADD COLUMN random_value int;
+```
+
+Add a column _with_ a default however does require downtime. For example,
+consider this query:
+
+```sql
+ALTER TABLE projects ADD COLUMN random_value int DEFAULT 42;
+```
+
+This requires updating every single row in the `projects` table so that
+`random_value` is set to `42` by default. This requires updating all rows and
+indexes in a table. This in turn acquires enough locks on the table for it to
+effectively block any other queries.
+
+As of MySQL 5.6 adding a column to a table is still quite an expensive
+operation, even when using `ALGORITHM=INPLACE` and `LOCK=NONE`. This means
+downtime _may_ be required when modifying large tables as otherwise the
+operation could potentially take hours to complete.
+
+Adding a column with a default value _can_ be done without requiring downtime
+when using the migration helper method
+`Gitlab::Database::MigrationHelpers#add_column_with_default`. This method works
+similar to `add_column` except it updates existing rows in batches without
+blocking access to the table being modified. See ["Adding Columns With Default
+Values"](migration_style_guide.html#adding-columns-with-default-values) for more
+information on how to use this method.
+
+## Dropping Columns
+
+On PostgreSQL you can safely remove an existing column without the need for
+downtime. When you drop a column in PostgreSQL it's not immediately removed,
+instead it is simply disabled. The data is removed on the next vacuum run.
+
+On MySQL this operation requires downtime.
+
+While database wise dropping a column may be fine on PostgreSQL this operation
+still requires downtime because the application code may still be using the
+column that was removed. For example, consider the following migration:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+ def change
+ remove_column :projects, :dummy
+ end
+end
+```
+
+Now imagine that the GitLab instance is running and actively uses the `dummy`
+column. If we were to run the migration this would result in the GitLab instance
+producing errors whenever it tries to use the `dummy` column.
+
+As a result of the above downtime _is_ required when removing a column, even
+when using PostgreSQL.
+
+## Changing Column Constraints
+
+Generally changing column constraints requires checking all rows in the table to
+see if they meet the new constraint, unless a constraint is _removed_. For
+example, changing a column that previously allowed NULL values to not allow NULL
+values requires the database to verify all existing rows.
+
+The specific behaviour varies a bit between databases but in general the safest
+approach is to assume changing constraints requires downtime.
+
+## Changing Column Types
+
+This operation requires downtime.
+
+## Adding Indexes
+
+Adding indexes is an expensive process that blocks INSERT and UPDATE queries for
+the duration. When using PostgreSQL one can work arounds this by using the
+`CONCURRENTLY` option:
+
+```sql
+CREATE INDEX CONCURRENTLY index_name ON projects (column_name);
+```
+
+Migrations can take advantage of this by using the method
+`add_concurrent_index`. For example:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+ def change
+ add_concurrent_index :projects, :column_name
+ end
+end
+```
+
+When running this on PostgreSQL the `CONCURRENTLY` option mentioned above is
+used. On MySQL this method produces a regular `CREATE INDEX` query.
+
+MySQL doesn't really have a workaround for this. Supposedly it _can_ create
+indexes without the need for downtime but only for variable width columns. The
+details on this are a bit sketchy. Since it's better to be safe than sorry one
+should assume that adding indexes requires downtime on MySQL.
+
+## Dropping Indexes
+
+Dropping an index does not require downtime on both PostgreSQL and MySQL.
+
+## Adding Tables
+
+This operation is safe as there's no code using the table just yet.
+
+## Dropping Tables
+
+This operation requires downtime as application code may still be using the
+table.
+
+## Adding Foreign Keys
+
+Adding foreign keys acquires an exclusive lock on both the source and target
+tables in PostgreSQL. This requires downtime as otherwise the entire application
+grinds to a halt for the duration of the operation.
+
+On MySQL this operation also requires downtime _unless_ foreign key checks are
+disabled. Because this means checks aren't enforced this is not ideal, as such
+one should assume MySQL also requires downtime.
+
+## Removing Foreign Keys
+
+This operation should not require downtime on both PostgreSQL and MySQL.
+
+## Updating Data
+
+Updating data should generally be safe. The exception to this is data that's
+being migrated from one version to another while the application still produces
+data in the old version.
+
+For example, imagine the application writes the string `'dog'` to a column but
+it really is meant to write `'cat'` instead. One might think that the following
+migration is all that is needed to solve this problem:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+ def up
+ execute("UPDATE some_table SET column = 'cat' WHERE column = 'dog';")
+ end
+end
+```
+
+Unfortunately this is not enough. Because the application is still running and
+using the old value this may result in the table still containing rows where
+`column` is set to `dog`, even after the migration finished.
+
+In these cases downtime _is_ required, even for rarely updated tables.