diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2016-11-30 12:21:33 +0100 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2016-11-30 12:21:33 +0100 |
commit | 00ca7adca2de8ff05cca3df9eb2df8a67f638cfe (patch) | |
tree | 86764da0f493a7169b2fea59e5618b669cd64d56 /doc/development | |
parent | adb3f3d4e494e8f8d41c1b9e676e395a49cd96b2 (diff) | |
parent | 7e5fa10b665835e3160eee4d333a17fbaef9c113 (diff) | |
download | gitlab-ce-00ca7adca2de8ff05cca3df9eb2df8a67f638cfe.tar.gz |
Merge branch 'master' into fix/rename-mwbs-to-merge-when-pipeline-succeeds
* master: (110 commits)
Rewrite an HTTP link to use HTTPS
Edit /spec/features/profiles/preferences_spec.rb to match changes in 084d90ac
Add blue back to sub nav active
Remove JSX/React eslint plugins.
Fix a transient spec failure
Adds hoverstates for collapsed Issue/Merge Request sidebar
Moved groups above projects
Add StackProf to the Gemfile, along with a utility to get a profile for a spec
Update Sidekiq-cron to fix compatibility issues with Sidekiq 4.2.1
Add a CHANGELOG entry
Alert user when logged in user email is not the same as the invitation
Expose timestamp in build entity used by serializer
Rename `MergeRequest#pipeline` to `head_pipeline`
Remove unnecessary database indexes
CE-specific changes gitlab-org/gitlab-ee#1137
Fixing typo & Clarifying Key name
fix started_at check
fix blob controller spec failure - updated not to use file-path-
fix blob controller spec failure
Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security'
...
Conflicts:
app/controllers/projects/merge_requests_controller.rb
lib/api/merge_requests.rb
spec/requests/api/merge_requests_spec.rb
Diffstat (limited to 'doc/development')
-rw-r--r-- | doc/development/doc_styleguide.md | 71 | ||||
-rw-r--r-- | doc/development/limit_ee_conflicts.md | 111 | ||||
-rw-r--r-- | doc/development/performance.md | 110 | ||||
-rw-r--r-- | doc/development/ux_guide/components.md | 20 | ||||
-rw-r--r-- | doc/development/ux_guide/img/components-anchorlinks.png | bin | 19948 -> 30089 bytes |
5 files changed, 270 insertions, 42 deletions
diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index b137e6ae82e..fc948a7a116 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -113,6 +113,77 @@ merge request. add an alternative text: `[identifier]: https://example.com "Alternative text"` that appears when hovering your mouse on a link +### Linking to inline docs + +Sometimes it's needed to link to the built-in documentation that GitLab provides +under `/help`. This is normally done in files inside the `app/views/` directory +with the help of the `help_page_path` helper method. + +In its simplest form, the HAML code to generate a link to the `/help` page is: + +```haml += link_to 'Help page', help_page_path('user/permissions') +``` + +The `help_page_path` contains the path to the document you want to link to with +the following conventions: + +- it is relative to the `doc/` directory in the GitLab repository +- the `.md` extension must be omitted +- it must not end with a slash (`/`) + +Below are some special cases where should be used depending on the context. +You can combine one or more of the following: + +1. **Linking to an anchor link.** Use `anchor` as part of the `help_page_path` + method: + + ```haml + = link_to 'Help page', help_page_path('user/permissions', anchor: 'anchor-link') + ``` + +1. **Opening links in a new tab.** This should be the default behavior: + + ```haml + = link_to 'Help page', help_page_path('user/permissions'), target: '_blank' + ``` + +1. **Linking to a circle icon.** Usually used in settings where a long + description cannot be used, like near checkboxes. You can basically use + any font awesome icon, but prefer the `question-circle`: + + ```haml + = link_to icon('question-circle'), help_page_path('user/permissions') + ``` + +1. **Using a button link.** Useful in places where text would be out of context + with the rest of the page layout: + + ```haml + = link_to 'Help page', help_page_path('user/permissions'), class: 'btn btn-info' + ``` + +1. **Underlining a link.** + + ```haml + = link_to 'Help page', help_page_path('user/permissions'), class: 'underlined-link' + ``` + +1. **Using links inline of some text.** + + ```haml + Description to #{link_to 'Help page', help_page_path('user/permissions')}. + ``` + +1. **Adding a period at the end of the sentence.** Useful when you don't want + the period to be part of the link: + + ```haml + = succeed '.' do + Learn more in the + = link_to 'Help page', help_page_path('user/permissions') + ``` + ## Images - Place images in a separate directory named `img/` in the same directory where diff --git a/doc/development/limit_ee_conflicts.md b/doc/development/limit_ee_conflicts.md index b7e6387838e..568dedf1669 100644 --- a/doc/development/limit_ee_conflicts.md +++ b/doc/development/limit_ee_conflicts.md @@ -143,109 +143,162 @@ to resolve when you add the indentation to the equation. For instance this kind of thing: ```haml +.form-group.detail-page-description + = form.label :description, 'Description', class: 'control-label' + .col-sm-10 + = render layout: 'projects/md_preview', locals: { preview_class: "md-preview", referenced_users: true } do + = render 'projects/zen', f: form, attr: :description, + classes: 'note-textarea', + placeholder: "Write a comment or drag your files here...", + supports_slash_commands: !issuable.persisted? + = render 'projects/notes/hints', supports_slash_commands: !issuable.persisted? + .clearfix + .error-alert +- if issuable.is_a?(Issue) + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = form.label :confidential do + = form.check_box :confidential + This issue is confidential and should only be visible to team members with at least Reporter access. - if can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) - has_due_date = issuable.has_attribute?(:due_date) %hr .row %div{ class: (has_due_date ? "col-lg-6" : "col-sm-12") } .form-group.issue-assignee - = f.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" + = form.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" .col-sm-10{ class: ("col-lg-8" if has_due_date) } .issuable-form-select-holder - if issuable.assignee_id - = f.hidden_field :assignee_id + = form.hidden_field :assignee_id = dropdown_tag(user_dropdown_label(issuable.assignee_id, "Assignee"), options: { toggle_class: "js-dropdown-keep-input js-user-search js-issuable-form-dropdown js-assignee-search", title: "Select assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit", placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) .form-group.issue-milestone - = f.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" + = form.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" .col-sm-10{ class: ("col-lg-8" if has_due_date) } .issuable-form-select-holder = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" .form-group - has_labels = @labels && @labels.any? - = f.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" - = f.hidden_field :label_ids, multiple: true, value: '' + = form.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" + = form.hidden_field :label_ids, multiple: true, value: '' .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false, show_menu_above: 'true' }, dropdown_title: "Select label" - + = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" - if issuable.respond_to?(:weight) + - weight_options = Issue.weight_options + - weight_options.delete(Issue::WEIGHT_ALL) + - weight_options.delete(Issue::WEIGHT_ANY) .form-group - = f.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do + = form.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do Weight .col-sm-10{ class: ("col-lg-8" if has_due_date) } - = f.select :weight, issues_weight_options(issuable.weight, edit: true), { include_blank: true }, - { class: 'select2 js-select2', data: { placeholder: "Select weight" }} - + .issuable-form-select-holder + - if issuable.weight + = form.hidden_field :weight + = dropdown_tag(issuable.weight || "Weight", options: { title: "Select weight", toggle_class: 'js-weight-select js-issuable-form-weight', dropdown_class: "dropdown-menu-selectable dropdown-menu-weight", + placeholder: "Search weight", data: { field_name: "#{issuable.class.model_name.param_key}[weight]" , default_label: "Weight" } }) do + %ul + - weight_options.each do |weight| + %li + %a{href: "#", data: { id: weight, none: weight === Issue::WEIGHT_NONE }, class: ("is-active" if issuable.weight == weight)} + = weight - if has_due_date .col-lg-6 .form-group - = f.label :due_date, "Due date", class: "control-label" + = form.label :due_date, "Due date", class: "control-label" .col-sm-10 .issuable-form-select-holder - = f.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" + = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" ``` could be simplified by using partials: ```haml -= render 'metadata_form', issuable: issuable += render 'shared/issuable/form/description', issuable: issuable, form: form + +- if issuable.respond_to?(:confidential) + .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = form.label :confidential do + = form.check_box :confidential + This issue is confidential and should only be visible to team members with at least Reporter access. + += render 'shared/issuable/form/metadata', issuable: issuable, form: form ``` -and then the `_metadata_form.html.haml` could be as follows: +and then the `app/views/shared/issuable/form/_metadata.html.haml` could be as follows: ```haml +- issuable = local_assigns.fetch(:issuable) + - return unless can?(current_user, :"admin_#{issuable.to_ability_name}", issuable.project) - has_due_date = issuable.has_attribute?(:due_date) +- has_labels = @labels && @labels.any? +- form = local_assigns.fetch(:form) + %hr .row %div{ class: (has_due_date ? "col-lg-6" : "col-sm-12") } .form-group.issue-assignee - = f.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" + = form.label :assignee_id, "Assignee", class: "control-label #{"col-lg-4" if has_due_date}" .col-sm-10{ class: ("col-lg-8" if has_due_date) } .issuable-form-select-holder - if issuable.assignee_id - = f.hidden_field :assignee_id + = form.hidden_field :assignee_id = dropdown_tag(user_dropdown_label(issuable.assignee_id, "Assignee"), options: { toggle_class: "js-dropdown-keep-input js-user-search js-issuable-form-dropdown js-assignee-search", title: "Select assignee", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable dropdown-menu-assignee js-filter-submit", - placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) + placeholder: "Search assignee", data: { first_user: current_user.try(:username), null_user: true, current_user: true, project_id: issuable.project.try(:id), selected: issuable.assignee_id, field_name: "#{issuable.class.model_name.param_key}[assignee_id]", default_label: "Assignee"} }) .form-group.issue-milestone - = f.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" + = form.label :milestone_id, "Milestone", class: "control-label #{"col-lg-4" if has_due_date}" .col-sm-10{ class: ("col-lg-8" if has_due_date) } .issuable-form-select-holder = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" .form-group - has_labels = @labels && @labels.any? - = f.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" - = f.hidden_field :label_ids, multiple: true, value: '' + = form.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" + = form.hidden_field :label_ids, multiple: true, value: '' .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } .issuable-form-select-holder - = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false, show_menu_above: 'true' }, dropdown_title: "Select label" + = render "shared/issuable/label_dropdown", classes: ["js-issuable-form-dropdown"], selected: issuable.labels, data_options: { field_name: "#{issuable.class.model_name.param_key}[label_ids][]", show_any: false }, dropdown_title: "Select label" - = render 'weight_form', issuable: issuable, has_due_date: has_due_date + = render "shared/issuable/form/weight", issuable: issuable, form: form - if has_due_date .col-lg-6 .form-group - = f.label :due_date, "Due date", class: "control-label" + = form.label :due_date, "Due date", class: "control-label" .col-sm-10 .issuable-form-select-holder - = f.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" + = form.text_field :due_date, id: "issuable-due-date", class: "datepicker form-control", placeholder: "Select due date" ``` -and then the `_weight_form.html.haml` could be as follows: +and then the `app/views/shared/issuable/form/_weight.html.haml` could be as follows: ```haml +- issuable = local_assigns.fetch(:issuable) + - return unless issuable.respond_to?(:weight) - has_due_date = issuable.has_attribute?(:due_date) +- form = local_assigns.fetch(:form) .form-group - = f.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do + = form.label :label_ids, class: "control-label #{"col-lg-4" if has_due_date}" do Weight .col-sm-10{ class: ("col-lg-8" if has_due_date) } - = f.select :weight, issues_weight_options(issuable.weight, edit: true), { include_blank: true }, - { class: 'select2 js-select2', data: { placeholder: "Select weight" }} + .issuable-form-select-holder + - if issuable.weight + = form.hidden_field :weight + + = weight_dropdown_tag(issuable, toggle_class: 'js-issuable-form-weight') do + %ul + - Issue.weight_options.each do |weight| + %li + %a{ href: '#', data: { id: weight, none: weight === Issue::WEIGHT_NONE }, class: ("is-active" if issuable.weight == weight) } + = weight ``` Note: diff --git a/doc/development/performance.md b/doc/development/performance.md index 8337c2d9cb3..5c43ae7b79a 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -101,6 +101,116 @@ In short: 5. If you must write a benchmark use the benchmark-ips Gem instead of Ruby's `Benchmark` module. +## Profiling + +By collecting snapshots of process state at regular intervals, profiling allows +you to see where time is spent in a process. The [StackProf](https://github.com/tmm1/stackprof) +gem is included in GitLab's development environment, allowing you to investigate +the behaviour of suspect code in detail. + +It's important to note that profiling an application *alters its performance*, +and will generally be done *in an unrepresentative environment*. In particular, +a method is not necessarily troublesome just because it is executed many times, +or takes a long time to execute. Profiles are tools you can use to better +understand what is happening in an application - using that information wisely +is up to you! + +Keeping that in mind, to create a profile, identify (or create) a spec that +exercises the troublesome code path, then run it using the `bin/rspec-stackprof` +helper, e.g.: + +``` +$ LIMIT=10 bin/rspec-stackprof spec/policies/project_policy_spec.rb +8/8 |====== 100 ======>| Time: 00:00:18 + +Finished in 18.19 seconds (files took 4.8 seconds to load) +8 examples, 0 failures + +================================== + Mode: wall(1000) + Samples: 17033 (5.59% miss rate) + GC: 1901 (11.16%) +================================== + TOTAL (pct) SAMPLES (pct) FRAME + 6000 (35.2%) 2566 (15.1%) Sprockets::Cache::FileStore#get + 2018 (11.8%) 888 (5.2%) ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#exec_no_cache + 1338 (7.9%) 640 (3.8%) ActiveRecord::ConnectionAdapters::PostgreSQL::DatabaseStatements#execute + 3125 (18.3%) 394 (2.3%) Sprockets::Cache::FileStore#safe_open + 913 (5.4%) 301 (1.8%) ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#exec_cache + 288 (1.7%) 288 (1.7%) ActiveRecord::Attribute#initialize + 246 (1.4%) 246 (1.4%) Sprockets::Cache::FileStore#safe_stat + 295 (1.7%) 193 (1.1%) block (2 levels) in class_attribute + 187 (1.1%) 187 (1.1%) block (4 levels) in class_attribute +``` + +You can limit the specs that are run by passing any arguments `rspec` would +normally take. + +The output is sorted by the `Samples` column by default. This is the number of +samples taken where the method is the one currently being executed. The `Total` +column shows the number of samples taken where the method, or any of the methods +it calls, were being executed. + +To create a graphical view of the call stack: + +```shell +$ stackprof tmp/project_policy_spec.rb.dump --graphviz > project_policy_spec.dot +$ dot -Tsvg project_policy_spec.dot > project_policy_spec.svg +``` + +To load the profile in [kcachegrind](https://kcachegrind.github.io/): + +``` +$ stackprof tmp/project_policy_spec.dump --callgrind > project_policy_spec.callgrind +$ kcachegrind project_policy_spec.callgrind # Linux +$ qcachegrind project_policy_spec.callgrind # Mac +``` + +It may be useful to zoom in on a specific method, e.g.: + +``` +$ stackprof tmp/project_policy_spec.rb.dump --method warm_asset_cache +TestEnv#warm_asset_cache (/Users/lupine/dev/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/spec/support/test_env.rb:164) + samples: 0 self (0.0%) / 6288 total (36.9%) + callers: + 6288 ( 100.0%) block (2 levels) in <top (required)> + callees (6288 total): + 6288 ( 100.0%) Capybara::RackTest::Driver#visit + code: + | 164 | def warm_asset_cache + | 165 | return if warm_asset_cache? + | 166 | return unless defined?(Capybara) + | 167 | + 6288 (36.9%) | 168 | Capybara.current_session.driver.visit '/' + | 169 | end +$ stackprof tmp/project_policy_spec.rb.dump --method BasePolicy#abilities +BasePolicy#abilities (/Users/lupine/dev/gitlab.com/gitlab-org/gitlab-development-kit/gitlab/app/policies/base_policy.rb:79) + samples: 0 self (0.0%) / 50 total (0.3%) + callers: + 25 ( 50.0%) BasePolicy.abilities + 25 ( 50.0%) BasePolicy#collect_rules + callees (50 total): + 25 ( 50.0%) ProjectPolicy#rules + 25 ( 50.0%) BasePolicy#collect_rules + code: + | 79 | def abilities + | 80 | return RuleSet.empty if @user && @user.blocked? + | 81 | return anonymous_abilities if @user.nil? + 50 (0.3%) | 82 | collect_rules { rules } + | 83 | end +``` + +Since the profile includes the work done by the test suite as well as the +application code, these profiles can be used to investigate slow tests as well. +However, for smaller runs (like this example), this means that the cost of +setting up the test suite will tend to dominate. + +It's also possible to modify the application code in-place to output profiles +whenever a particular code path is triggered without going through the test +suite first. See the +[StackProf documentation](https://github.com/tmm1/stackprof/blob/master/README.md) +for details. + ## Importance of Changes When working on performance improvements, it's important to always ask yourself diff --git a/doc/development/ux_guide/components.md b/doc/development/ux_guide/components.md index 764c3355714..8e51edd23ef 100644 --- a/doc/development/ux_guide/components.md +++ b/doc/development/ux_guide/components.md @@ -43,7 +43,7 @@ Primary links are blue in their rest state. Secondary links (such as the time st #### Hover -An underline should always be added on hover. A gray link becomes blue on hover. +On hover, an underline should be added and the color should change. Both the primary and secondary link should become the darker blue color on hover. #### Focus @@ -72,9 +72,7 @@ Secondary buttons are for alternative commands. They should be conveyed by a bu ### Icon and text treatment Text should be in sentence case, where only the first word is capitalized. "Create issue" is correct, not "Create Issue". Buttons should only contain an icon or a text, not both. ->>> -TODO: Rationalize this. Ensure that we still believe this. ->>> +> TODO: Rationalize this. Ensure that we still believe this. ### Colors Follow the color guidance on the [basics](basics.md#color) page. The default color treatment is the white/grey button. @@ -85,13 +83,13 @@ Follow the color guidance on the [basics](basics.md#color) page. The default col Dropdowns are used to allow users to choose one (or many) options from a list of options. If this list of options is more 20, there should generally be a way to search through and filter the options (see the complex filter dropdowns below.) ->>> -TODO: Will update this section when the new filters UI is implemented. ->>> +> TODO: Will update this section when the new filters UI is implemented. ![Dropdown states](img/components-dropdown.png) +### Max size +The max height for dropdowns should target **10-15 items**. If the height of the dropdown is too large, the list becomes very hard to parse and it is easy to visually lose track of the item you are looking for. Usability also suffers as more mouse movement is required, and you have a larger area in which you hijack the scroll away from the page level. While it may initially seem counterintuitive to not show as many items as you can, it is actually quicker and easier to process the information when it is cropped at a reasonable height. --- @@ -164,9 +162,7 @@ Cover blocks are generally used to create a heading element for a page, such as ## Panels ->>> -TODO: Catalog how we are currently using panels and rationalize how they relate to alerts ->>> +> TODO: Catalog how we are currently using panels and rationalize how they relate to alerts ![Panels](img/components-panels.png) @@ -174,9 +170,7 @@ TODO: Catalog how we are currently using panels and rationalize how they relate ## Alerts ->>> -TODO: Catalog how we are currently using alerts ->>> +> TODO: Catalog how we are currently using alerts ![Alerts](img/components-alerts.png) diff --git a/doc/development/ux_guide/img/components-anchorlinks.png b/doc/development/ux_guide/img/components-anchorlinks.png Binary files differindex 7dd6a8a3876..4a9c730566c 100644 --- a/doc/development/ux_guide/img/components-anchorlinks.png +++ b/doc/development/ux_guide/img/components-anchorlinks.png |