diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2018-05-04 16:58:47 +0300 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-05-04 16:58:47 +0300 |
commit | 1983356d647290fe38ca21bbbca43fe2d6292913 (patch) | |
tree | d07fba5693e239993dfc6d1f724b2103f90a3fa6 /doc/development/fe_guide | |
parent | 703f45632292e7fc45359d0144cd616725bf9b0d (diff) | |
parent | 4bf47cd76fd69a26b7b2b4ac029f088ec5493712 (diff) |
Merge branch 'master' into 44427-state-management-with-vuext
* master: (1063 commits)
Replace commits spinach tests with RSpec analog
Update repository.rb
Add note about rebase/squash duplication in Gitaly
Resolve "Reconcile project templates with Auto DevOps"
Move import project pane to a separate partial
Inform the user when there are no project import options available
Clarify location of Vue templates
Make add_index_to_namespaces_runners_token migration reversible
Fix lambda arguments in Grape entities
Update grape-entity 0.6.0 -> 0.7.1
Fix constants in backfill_runner_type_for_ci_runners_post_migrate.rb
Use limited_counter_with_delimiter in the admin user list tabs
Remove a warning from spec/features/admin/admin_users_spec.rb
Use smallint for runner_type since its an enum
Dont remove duplicates in Runner.owned_or_shared since its not necessary
Change the docs license to CC BY-SA
Remove unnecessary disable transaction in add_ci_runner_namespaces
Split migration to add and index namespaces.runners_token
Output some useful information when running the rails console
Revert "Use factory in specs for ProjectCiCdSettings"
...
Diffstat (limited to 'doc/development/fe_guide')
-rw-r--r-- | doc/development/fe_guide/development_process.md | 77 | ||||
-rw-r--r-- | doc/development/fe_guide/icons.md | 2 | ||||
-rw-r--r-- | doc/development/fe_guide/index.md | 52 | ||||
-rw-r--r-- | doc/development/fe_guide/style_guide_js.md | 10 | ||||
-rw-r--r-- | doc/development/fe_guide/vue.md | 1 | ||||
-rw-r--r-- | doc/development/fe_guide/vuex.md | 6 |
6 files changed, 99 insertions, 49 deletions
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. + + ![Diagram of Issue Boards Architecture](img/boards_diagram.png) + +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/icons.md b/doc/development/fe_guide/icons.md index b288ee95722..b469a9c6aef 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -49,7 +49,7 @@ Please use the following function inside JS to render an icon : All Icons and Illustrations are managed in the [gitlab-svgs](https://gitlab.com/gitlab-org/gitlab-svgs) repository which is added as a dev-dependency. -To upgrade to a new SVG Sprite version run `yarn upgrade @gitlab-org/gitlab-svgs` and then run `yarn run svg`. This task will copy the svg sprite and all illustrations in the correct folders. The updated files should be tracked in Git as those are referenced. +To upgrade to a new SVG Sprite version run `yarn upgrade @gitlab-org/gitlab-svgs`. # SVG Illustrations diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 73084d667d4..5a67414e835 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,66 +22,32 @@ 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. - - ![Diagram of Issue Boards Architecture](img/boards_diagram.png) - -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. ---- - ## [Vuex](vuex.md) Vuex specific design patterns and practices. ---- - ## [Axios](axios.md) Axios specific practices and gotchas. diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 7b5fa6ca42f..04dfe418dbe 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -236,7 +236,7 @@ export class Foo { } ``` -On the other hand, if a class only needs to extend a third party/add event listeners in some specific cases, they should be initialized oustside of the constructor. +On the other hand, if a class only needs to extend a third party/add event listeners in some specific cases, they should be initialized outside of the constructor. 1. Prefer `.map`, `.reduce` or `.filter` over `.forEach` A forEach will most likely cause side effects, it will be mutating the array being iterated. Prefer using `.map`, @@ -310,7 +310,7 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. })); ``` -1. Don not use a singleton for the service or the store +1. Do not use a singleton for the service or the store ```javascript // bad class Store { @@ -328,9 +328,11 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. } } ``` +1. Use `.vue` for Vue templates. Do not use `%template` in HAML. #### Naming -1. **Extensions**: Use `.vue` extension for Vue components. + +1. **Extensions**: Use `.vue` extension for Vue components. Do not use `.js` as file extension ([#34371]). 1. **Reference Naming**: Use PascalCase for their instances: ```javascript // bad @@ -364,6 +366,8 @@ Please check this [rules][eslint-plugin-vue-rules] for more documentation. <component my-prop="prop" /> ``` +[#34371]: https://gitlab.com/gitlab-org/gitlab-ce/issues/34371 + #### Alignment 1. Follow these alignment styles for the template method: 1. With more than one attribute, all attributes should be on a new line: diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 3b68fd858cc..62c3a05eb3b 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -440,7 +440,6 @@ need to test the rendered output. [Vue][vue-test] guide's to unit test show us e Refer to [mock axios](axios.md#mock-axios-response-on-tests) - [vue-docs]: http://vuejs.org/guide/index.html [issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards [environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 7298ffcd54a..d0d74afe7bb 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -1,5 +1,5 @@ # Vuex -To manage the state of an application you may use [Vuex][vuex-docs]. +To manage the state of an application you should use [Vuex][vuex-docs]. _Note:_ All of the below is explained in more detail in the official [Vuex documentation][vuex-docs]. @@ -115,8 +115,8 @@ create: 1. An action `requestSomething`, to toggle the loading state 1. An action `receiveSomethingSuccess`, to handle the success callback 1. An action `receiveSomethingError`, to handle the error callback -1. An action `fetchSomething` to make the request. - 1. In case your application does more than a `GET` request you can use these as examples: +1. An action `fetchSomething` to make the request. + 1. In case your application does more than a `GET` request you can use these as examples: 1. `PUT`: `createSomething` 2. `POST`: `updateSomething` 3. `DELETE`: `deleteSomething` |