summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-02-29 11:00:33 +0000
committerDouwe Maan <douwe@gitlab.com>2016-02-29 11:00:33 +0000
commit17251cf447c5a7d2823d892e6dc1c4b27a268a25 (patch)
tree9b6d665947fce01d96aa41cca473f750e5c10ae6
parentf838dbe4d13c55fc8796eadf33165f1406612662 (diff)
parent0afe2e051332dbcf69804576ad326cfc4627fc79 (diff)
downloadgitlab-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
-rw-r--r--doc/development/README.md11
-rw-r--r--doc/development/gotchas.md103
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)