From 6053ad847400443e8ab870cc6a4bcb14be236fb3 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Tue, 16 Feb 2016 22:09:00 -0500
Subject: [PATCH] Add "Gotchas" development doc

---
 doc/development/README.md  |   1 +
 doc/development/gotchas.md | 104 +++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 doc/development/gotchas.md

diff --git a/doc/development/README.md b/doc/development/README.md
index 4226dc3df12..b9a0d81e5ba 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -3,6 +3,7 @@
 - [Architecture](architecture.md) of GitLab
 - [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
diff --git a/doc/development/gotchas.md b/doc/development/gotchas.md
new file mode 100644
index 00000000000..d2d8a6119dd
--- /dev/null
+++ b/doc/development/gotchas.md
@@ -0,0 +1,104 @@
+# 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.
+
+We've [removed these two filters entirely](https://gitlab.com/gitlab-org/gitlab-ce/blob/8-5-stable/config/initializers/haml.rb)
+through 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();
+```
+
+But there's a feature of GitLab's Markdown processing that will automatically
+add `id` attributes underneath header elements in order to make them linkable.
+The content of the header is ["dasherized"][ToC Processing] and used in the `id`
+attribute.
+
+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 JavaScript with the following Markdown:
+
+```markdown
+## JS My Selector
+```
+
+Which gets converted to the following HTML after processing:
+
+```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)