diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-02-29 11:00:33 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2016-02-29 11:00:33 +0000 |
commit | 17251cf447c5a7d2823d892e6dc1c4b27a268a25 (patch) | |
tree | 9b6d665947fce01d96aa41cca473f750e5c10ae6 /doc/development | |
parent | f838dbe4d13c55fc8796eadf33165f1406612662 (diff) | |
parent | 0afe2e051332dbcf69804576ad326cfc4627fc79 (diff) | |
download | gitlab-ce-17251cf447c5a7d2823d892e6dc1c4b27a268a25.tar.gz |
Merge branch 'rs-development-docs' into 'master'
Add "Gotchas" to development docs
Also:
- Alphabetizes the "Developer" index page
- Fixes the broken `db_dump.md` link
See merge request !2846
Diffstat (limited to 'doc/development')
-rw-r--r-- | doc/development/README.md | 11 | ||||
-rw-r--r-- | doc/development/gotchas.md | 103 |
2 files changed, 109 insertions, 5 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index d5bf166ad32..b9a0d81e5ba 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -1,11 +1,12 @@ # Development - [Architecture](architecture.md) of GitLab -- [Shell commands](shell_commands.md) in the GitLab codebase -- [Rake tasks](rake_tasks.md) for development +- [Benchmarking](benchmarking.md) - [CI setup](ci_setup.md) for testing GitLab +- [Gotchas](gotchas.md) to avoid +- [How to dump production data to staging](db_dump.md) +- [Migration Style Guide](migration_style_guide.md) for creating safe migrations +- [Rake tasks](rake_tasks.md) for development +- [Shell commands](shell_commands.md) in the GitLab codebase - [Sidekiq debugging](sidekiq_debugging.md) - [UI guide](ui_guide.md) for building GitLab with existing css styles and elements -- [Migration Style Guide](migration_style_guide.md) for creating safe migrations -- [How to dump production data to staging](dump_db.md) -- [Benchmarking](benchmarking.md) diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md new file mode 100644 index 00000000000..21078c8d6f9 --- /dev/null +++ b/doc/development/gotchas.md @@ -0,0 +1,103 @@ +# Gotchas + +The purpose of this guide is to document potential "gotchas" that contributors +might encounter or should avoid during development of GitLab CE and EE. + +## Don't `describe` symbols + +Consider the following model spec: + +```ruby +require 'rails_helper' + +describe User do + describe :to_param do + it 'converts the username to a param' do + user = described_class.new(username: 'John Smith') + + expect(user.to_param).to eq 'john-smith' + end + end +end +``` + +When run, this spec doesn't do what we might expect: + +```sh +spec/models/user_spec.rb|6 error| Failure/Error: u = described_class.new NoMethodError: undefined method `new' for :to_param:Symbol +``` + +### Solution + +Except for the top-level `describe` block, always provide a String argument to +`describe`. + +## Don't `rescue Exception` + +See ["Why is it bad style to `rescue Exception => e` in Ruby?"][Exception]. + +_**Note:** This rule is [enforced automatically by +Rubocop](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/.rubocop.yml#L911-914)._ + +[Exception]: http://stackoverflow.com/q/10048173/223897 + +## Don't use inline CoffeeScript in views + +Using the inline `:coffee` or `:coffeescript` Haml filters comes with a +performance overhead. + +_**Note:** We've [removed these two filters](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-5-stable/config/initializers/haml.rb) +in an initializer._ + +### Further reading + +- Pull Request: [Replace CoffeeScript block into JavaScript in Views](https://git.io/vztMu) +- 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 + +Normally, because HTML `id` attributes need to be unique to the page, it's +perfectly fine to write some JavaScript like the following: + +```javascript +$('#js-my-selector').hide(); +``` + +However, there's a feature of GitLab's Markdown processing that [automatically +adds anchors to header elements][ToC Processing], with the `id` attribute being +automatically generated based on the content of the header. + +Unfortunately, this feature makes it possible for user-generated content to +create a header element with the same `id` attribute we're using in our +selector, potentially breaking the JavaScript behavior. A user could break the +above example with the following Markdown: + +```markdown +## JS My Selector +``` + +Which gets converted to the following HTML: + +```html +<h2> + <a id="js-my-selector" class="anchor" href="#js-my-selector" aria-hidden="true"></a> + JS My Selector +</h2> +``` + +[ToC Processing]: https://gitlab.com/gitlab-org/gitlab-ce/blob/8-4-stable/lib/banzai/filter/table_of_contents_filter.rb#L31-37 + +### Solution + +The current recommended fix for this is to make our selectors slightly more +specific: + +```javascript +$('div#js-my-selector').hide(); +``` + +### Further reading + +- Issue: [Merge request ToC anchor conflicts with tabs](https://gitlab.com/gitlab-org/gitlab-ce/issues/3908) +- Merge Request: [Make tab target selectors less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2023) +- Merge Request: [Make cross-project reference's clipboard target less naive](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2024) |