diff options
Diffstat (limited to 'doc/development')
-rw-r--r-- | doc/development/background_migrations.md | 15 | ||||
-rw-r--r-- | doc/development/changelog.md | 2 | ||||
-rw-r--r-- | doc/development/doc_styleguide.md | 33 | ||||
-rw-r--r-- | doc/development/fe_guide/development_process.md | 77 | ||||
-rw-r--r-- | doc/development/fe_guide/index.md | 50 | ||||
-rw-r--r-- | doc/development/fe_guide/vue.md | 12 | ||||
-rw-r--r-- | doc/development/i18n/proofreader.md | 1 | ||||
-rw-r--r-- | doc/development/testing_guide/best_practices.md | 36 | ||||
-rw-r--r-- | doc/development/testing_guide/end_to_end_tests.md | 6 | ||||
-rw-r--r-- | doc/development/testing_guide/testing_rake_tasks.md | 2 |
10 files changed, 176 insertions, 58 deletions
diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index fc1b202b5eb..ce69694ab6a 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -133,11 +133,19 @@ roughly be as follows: 1. Release B: 1. Deploy code so that the application starts using the new column and stops scheduling jobs for newly created data. - 1. In a post-deployment migration you'll need to ensure no jobs remain. To do - so you can use `Gitlab::BackgroundMigration.steal` to process any remaining - jobs before continuing. + 1. In a post-deployment migration you'll need to ensure no jobs remain. + 1. Use `Gitlab::BackgroundMigration.steal` to process any remaining + jobs in Sidekiq. + 1. Reschedule the migration to be run directly (i.e. not through Sidekiq) + on any rows that weren't migrated by Sidekiq. This can happen if, for + instance, Sidekiq received a SIGKILL, or if a particular batch failed + enough times to be marked as dead. 1. Remove the old column. +This may also require a bump to the [import/export version][import-export], if +importing a project from a prior version of GitLab requires the data to be in +the new format. + ## Example To explain all this, let's use the following example: the table `services` has a @@ -296,3 +304,4 @@ for more details. [migrations-readme]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/migrations/README.md [issue-rspec-hooks]: https://gitlab.com/gitlab-org/gitlab-ce/issues/35351 [reliable-sidekiq]: https://gitlab.com/gitlab-org/gitlab-ce/issues/36791 +[import-export]: ../user/project/settings/import_export.md diff --git a/doc/development/changelog.md b/doc/development/changelog.md index d5a4acff481..a9fa5ae834f 100644 --- a/doc/development/changelog.md +++ b/doc/development/changelog.md @@ -22,7 +22,7 @@ The `merge_request` value is a reference to a merge request that adds this entry, and the `author` key is used to give attribution to community contributors. **Both are optional**. The `type` field maps the category of the change, -valid options are: added, fixed, changed, deprecated, removed, security, other. **Type field is mandatory**. +valid options are: added, fixed, changed, deprecated, removed, security, performance, other. **Type field is mandatory**. Community contributors and core team members are encouraged to add their name to the `author` field. GitLab team members **should not**. diff --git a/doc/development/doc_styleguide.md b/doc/development/doc_styleguide.md index 41e3412c7ff..0550ea527cb 100644 --- a/doc/development/doc_styleguide.md +++ b/doc/development/doc_styleguide.md @@ -157,6 +157,39 @@ below. Otherwise, leave this mention out. +### Product badges + +When a feature is available in EE-only tiers, add the corresponding tier according to the +feature availability: + +- For GitLab Starter and GitLab.com Bronze: `**[STARTER]**` +- For GitLab Premium and GitLab.com Silver: `**[PREMIUM]**` +- For GitLab Ultimate and GitLab.com Gold: `**[ULTIMATE]**` +- For GitLab Core and GitLab.com Free: `**[CORE]**` + +To exclude GitLab.com tiers (when the feature is not available in GitLab.com), add the +keyword "only": + +- For GitLab Starter: `**[STARTER ONLY]**` +- For GitLab Premium: `**[PREMIUM ONLY]**` +- For GitLab Ultimate: `**[ULTIMATE ONLY]**` +- For GitLab Core: `**[CORE ONLY]**` + +The tier should be ideally added to headers, so that the full badge will be displayed. +But it can be also mentioned from paragraphs, list items, and table cells. For these cases, +the tier mention will be represented by an orange question mark. +E.g., `**[STARTER]**` renders **[STARTER]**, `**[STARTER ONLY]**` renders **[STARTER ONLY]**. + +The absence of tiers' mentions mean that the feature is available in GitLab Core, +GitLab.com Free, and higher tiers. + +#### How it works + +Introduced by [!244](https://gitlab.com/gitlab-com/gitlab-docs/merge_requests/244), +the special markup `**[STARTER]**` will generate a `span` element to trigger the +badges and tooltips (`<span class="badge-trigger starter">`). When the keyword +"only" is added, the corresponding GitLab.com badge will not be displayed. + ### GitLab Restart There are many cases that a restart/reconfigure of GitLab is required. To diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md new file mode 100644 index 00000000000..5504a6421d5 --- /dev/null +++ b/doc/development/fe_guide/development_process.md @@ -0,0 +1,77 @@ +# Frontend Development Process + +You can find more about the organization of the frontend team in the [handbook](https://about.gitlab.com/handbook/frontend/). + +## Development Checklist + +The idea is to remind us about specific topics during the time we build a new feature or start something. This is a common practice in other industries (like pilots) that also use standardised checklists to reduce problems early on. + +Copy the content over to your issue or merge request and if something doesn't apply simply remove it from your current list. + +This checklist is intended to help us during development of bigger features/refactorings, it's not a "use it always and every point always matches" list. + +Please use your best judgement when to use it and please contribute new points through merge requests if something comes to your mind. + +--- + +### Frontend development + +#### Planning development + +- [ ] Check the current set weight of the issue, does it fit your estimate? +- [ ] Are all [departments](https://about.gitlab.com/handbook/engineering/#engineering-teams) that are needed from your perspective already involved in the issue? (For example is UX missing?) +- [ ] Is the specification complete? Are you missing decisions? How about error handling/defaults/edge cases? Take your time to understand the needed implementation and go through its flow. +- [ ] Are all necessary UX specifications available that you will need in order to implement? Are there new UX components/patterns in the designs? Then contact the UI component team early on. How should error messages or validation be handled? +- [ ] **Library usage** Use Vuex as soon as you have even a medium state to manage, use Vue router if you need to have different views internally and want to link from the outside. Check what libraries we already have for which occassions. +- [ ] **Plan your implementation:** + - [ ] **Architecture plan:** Create a plan aligned with GitLab's architecture, how you are going to do the implementation, for example Vue application setup and its components (through [onion skinning](https://gitlab.com/gitlab-org/gitlab-ce/issues/35873#note_39994091)), Store structure and data flow, which existing Vue components can you reuse. Its a good idea to go through your plan with another engineer to refine it. + - [ ] **Backend:** The best way is to kickoff the implementation in a call and discuss with the assigned Backend engineer what you will need from the backend and also when. Can you reuse existing API's? How is the performance with the planned architecture? Maybe create together a JSON mock object to already start with development. + - [ ] **Communication:** It also makes sense to have for bigger features an own slack channel (normally called #f_{feature_name}) and even weekly demo calls with all people involved. + - [ ] **Dependency Plan:** Are there big dependencies in the plan between you and others, then maybe create an execution diagram to show what is blocking which part and the order of the different parts. + - [ ] **Task list:** Create a simple checklist of the subtasks that are needed for the implementation, also consider creating even sub issues. (for example show a comment, delete a comment, update a comment, etc.). This helps you and also everyone else following the implementation +- [ ] **Keep it small** To make it easier for you and also all reviewers try to keep merge requests small and merge into a feature branch if needed. To accomplish that you need to plan that from the start. Different methods are: + - [ ] **Skeleton based plan** Start with an MR that has the skeleton of the components with placeholder content. In following MRs you can fill the components with interactivity. This also makes it easier to spread out development on multiple people. + - [ ] **Cookie Mode** Think about hiding the feature behind a cookie flag if the implementation is on top of existing features + - [ ] **New route** Are you refactoring something big then you might consider adding a new route where you implement the new feature and when finished delete the current route and rename the new one. (for example 'merge_request' and 'new_merge_request') +- [ ] **Setup** Is there any specific setup needed for your implementation (for example a kubernetes cluster)? Then let everyone know if it is not already mentioned where they can find documentation (if it doesn't exist - create it) +- [ ] **Security** Are there any new security relevant implementations? Then please contact the security team for an app security review. If you are not sure ask our [domain expert](https://about.gitlab.com/handbook/frontend/#frontend-domain-experts) + +#### During development + +- [ ] Check off tasks on your created task list to keep everyone updated on the progress +- [ ] [Share your work early with reviewers/maintainers](#share-your-work-early) +- [ ] Share your work with UXer and Product Manager with Screenshots and/or [GIF's](https://about.gitlab.com/handbook/product/making-gifs/). They are easy to create for you and keep them up to date. +- [ ] If you are blocked on something let everyone on the issue know through a comment. +- [ ] Are you unable to work on this issue for a longer period of time, also let everyone know. +- [ ] **Documentation** Update/add docs for the new feature, see `docs/`. Ping one of the documentation experts/reviewers + +#### Finishing development + Review + +- [ ] **Keep it in the scope** Try to focus on the actual scope and avoid a scope creep during review and keep new things to new issues. +- [ ] **Performance** Have you checked performance? For example do the same thing with 500 comments instead of 1. Document the tests and possible findings in the MR so a reviewer can directly see it. +- [ ] Have you tested with a variety of our [supported browsers](../../install/requirements.md#supported-web-browsers)? You can use [browserstack](https://www.browserstack.com/) to be able to access a wide variety of browsers and operating systems. +- [ ] Did you check the mobile view? +- [ ] Check the built webpack bundle (For the report run `WEBPACK_REPORT=true gdk run`, then open `webpack-report/index.html`) if we have unnecessary bloat due to wrong references, including libraries multiple times, etc.. If you need help contact the webpack [domain expert](https://about.gitlab.com/handbook/frontend/#frontend-domain-experts) +- [ ] **Tests** Not only greenfield tests - Test also all bad cases that come to your mind. +- [ ] If you have multiple MR's then also smoke test against the final merge. +- [ ] Are there any big changes on how and especially how frequently we use the API then let production know about it +- [ ] Smoke test of the RC on dev., staging., canary deployments and .com +- [ ] Follow up on issues that came out of the review. Create isssues for discovered edge cases that should be covered in future iterations. + +--- + +### Share your work early +1. Before writing code, ensure your vision of the architecture is aligned with +GitLab's architecture. +1. Add a diagram to the issue and ask a frontend architect in the slack channel `#fe_architectural` about it. + +  + +1. Don't take more than one week between starting work on a feature and +sharing a Merge Request with a reviewer or a maintainer. + +### Vue features +1. Follow the steps in [Vue.js Best Practices](vue.md) +1. Follow the style guide. +1. Only a handful of people are allowed to merge Vue related features. +Reach out to one of Vue experts early in this process. diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 2280cf79f86..3b4dfd50761 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -5,11 +5,15 @@ across GitLab's frontend team. ## Overview -GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] with -[Hamlit][hamlit]. Be wary of [the limitations that come with using -Hamlit][hamlit-limits]. We also use [SCSS][scss] 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][rails] using [Haml][haml] and also a JavaScript based Frontend with [Vue.js][vue]. +Be wary of [the limitations that come with using Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with +modern ECMAScript standards supported through [Babel][babel] and ES module support through [webpack][webpack]. + +### Javascript development + +[Vue.js][vue] is used for particularly advanced, dynamic elements and based on previous iterations [jQuery][jquery] is used in lot of places through the application's JavaScript. + +We also use [Axios][axios] to handle all of our network requests. We also utilize [webpack][webpack] to handle the bundling, minification, and compression of our assets. @@ -18,61 +22,29 @@ Working with our frontend assets requires Node (v6.0 or greater) and Yarn (v1.2 or greater). You can find information on how to install these on our [installation guide][install]. -[jQuery][jquery] is used throughout the application's JavaScript, with -[Vue.js][vue] for particularly advanced, dynamic elements. - -We also use [Axios][axios] to handle all of our network requests. - ### Browser Support For our currently-supported browsers, see our [requirements][requirements]. --- -## Development Process - -### Share your work early -1. Before writing code guarantee your vision of the architecture is aligned with -GitLab's architecture. -1. Add a diagram to the issue and ask a Frontend Architecture about it. - -  - -1. Don't take more than one week between starting work on a feature and -sharing a Merge Request with a reviewer or a maintainer. - -### Vue features -1. Follow the steps in [Vue.js Best Practices](vue.md) -1. Follow the style guide. -1. Only a handful of people are allowed to merge Vue related features. -Reach out to one of Vue experts early in this process. - - ---- +## [Development Process](development_process.md) +How we plan and execute the work on the frontend. ## [Architecture](architecture.md) How we go about making fundamental design decisions in GitLab's frontend team or make changes to our frontend development guidelines. ---- - ## [Testing](../testing_guide/frontend_testing.md) - How we write frontend tests, run the GitLab test suite, and debug test related issues. ---- - ## [Design Patterns](design_patterns.md) Common JavaScript design patterns in GitLab's codebase. ---- - ## [Vue.js Best Practices](vue.md) Vue specific design patterns and practices. ---- - ## [Axios](axios.md) Axios specific practices and gotchas. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index c1170fa3b13..9c4b0e86351 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -523,8 +523,8 @@ export default new Vuex.Store({ ``` _Note:_ If the state of the application is too complex, an individual file for the state may be better. -#### `actions.js` -An action commits a mutatation. In this file, we will write the actions that will call the respective mutation: +##### `actions.js` +An action commits a mutation. In this file, we will write the actions that will commit the respective mutation: ```javascript import * as types from './mutation_types'; @@ -550,7 +550,7 @@ import { mapActions } from 'vuex'; }; ``` -#### `getters.js` +##### `getters.js` Sometimes we may need to get derived state based on store state, like filtering for a specific prop. This can be done through the `getters`: ```javascript @@ -573,7 +573,7 @@ import { mapGetters } from 'vuex'; }; ``` -#### `mutations.js` +##### `mutations.js` The only way to actually change state in a Vuex store is by committing a mutation. ```javascript @@ -586,7 +586,7 @@ The only way to actually change state in a Vuex store is by committing a mutatio }; ``` -#### `mutations_types.js` +##### `mutations_types.js` From [vuex mutations docs][vuex-mutations]: > It is a commonly seen pattern to use constants for mutation types in various Flux implementations. This allows the code to take advantage of tooling like linters, and putting all constants in a single file allows your collaborators to get an at-a-glance view of what mutations are possible in the entire application. @@ -661,7 +661,7 @@ describe('component', () => { }; // populate the store - store.dipatch('addUser', user); + store.dispatch('addUser', user); vm = new Component({ store, diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index cf62314bc29..5185d843ccb 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -23,6 +23,7 @@ are very appreciative of the work done by translators and proofreaders! - Italian - Paolo Falomo - [GitLab](https://gitlab.com/paolofalomo), [Crowdin](https://crowdin.com/profile/paolo.falomo) - Japanese + - Yamana Tokiuji - [GitLab](https://gitlab.com/tokiuji), [Crowdin](https://crowdin.com/profile/yamana) - Korean - Chang-Ho Cha - [GitLab](https://gitlab.com/changho-cha), [Crowdin](https://crowdin.com/profile/zzazang) - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index df80cd9f584..b77e9b7ff63 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -90,6 +90,25 @@ Finished in 34.51 seconds (files took 0.76702 seconds to load) Note: `live_debug` only works on javascript enabled specs. +### Fast unit tests + +Some classes are well-isolated from Rails and you should be able to test them +without the overhead added by the Rails environment and Bundler's `:default` +group's gem loading. In these cases, you can `require 'fast_spec_helper'` +instead of `require 'spec_helper'` in your test file, and your test should run +really fast since: + +- Gems loading is skipped +- Rails app boot is skipped +- gitlab-shell and Gitaly setup are skipped +- Test repositories setup are skipped + +Note that in some cases, you might have to add some `require_dependency 'foo'` +in your file under test since Rails autoloading is not available in these cases. + +This shouldn't be a problem since explicitely listing dependencies should be +considered a good practice anyway. + ### `let` variables GitLab's RSpec suite has made extensive use of `let` variables to reduce @@ -281,14 +300,13 @@ All fixtures should be be placed under `spec/fixtures/`. RSpec config files are files that change the RSpec config (i.e. `RSpec.configure do |config|` blocks). They should be placed under -`spec/support/config/`. +`spec/support/`. Each file should be related to a specific domain, e.g. -`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc. +`spec/support/capybara.rb`, `spec/support/carrierwave.rb`, etc. -Helpers can be included in the `spec/support/config/rspec.rb` file. If a -helpers module applies only to a certain kind of specs, it should add modifiers -to the `config.include` call. For instance if +If a helpers module applies only to a certain kind of specs, it should add +modifiers to the `config.include` call. For instance if `spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and `type: :model` specs only, you would write the following: @@ -299,6 +317,14 @@ RSpec.configure do |config| end ``` +If a config file only consists of `config.include`, you can add these +`config.include` directly in `spec/spec_helper.rb`. + +For very generic helpers, consider including them in the `spec/support/rspec.rb` +file which is used by the `spec/fast_spec_helper.rb` file. See +[Fast unit tests](#fast-unit-tests) for more details about the +`spec/fast_spec_helper.rb` file. + --- [Return to Testing documentation](index.md) diff --git a/doc/development/testing_guide/end_to_end_tests.md b/doc/development/testing_guide/end_to_end_tests.md index d10a797a142..21ec926414d 100644 --- a/doc/development/testing_guide/end_to_end_tests.md +++ b/doc/development/testing_guide/end_to_end_tests.md @@ -67,9 +67,9 @@ and examples in [the `qa/` directory][instance-qa-examples]. ## Where can I ask for help? -You can ask question in the `#qa` channel on Slack (GitLab internal) or you can -find an issue you would like to work on in [the issue tracker][gitlab-qa-issues] -and start a new discussion there. +You can ask question in the `#quality` channel on Slack (GitLab internal) or +you can find an issue you would like to work on in +[the issue tracker][gitlab-qa-issues] and start a new discussion there. [omnibus-gitlab]: https://gitlab.com/gitlab-org/omnibus-gitlab [gitlab-qa]: https://gitlab.com/gitlab-org/gitlab-qa diff --git a/doc/development/testing_guide/testing_rake_tasks.md b/doc/development/testing_guide/testing_rake_tasks.md index 60163f1a230..db8ca87e9f8 100644 --- a/doc/development/testing_guide/testing_rake_tasks.md +++ b/doc/development/testing_guide/testing_rake_tasks.md @@ -9,7 +9,7 @@ At a minimum, requiring the Rake helper will redirect `stdout`, include the runtime task helpers, and include the `RakeHelpers` Spec support module. The `RakeHelpers` module exposes a `run_rake_task(<task>)` method to make -executing tasks simple. See `spec/support/rake_helpers.rb` for all available +executing tasks simple. See `spec/support/helpers/rake_helpers.rb` for all available methods. Example: |