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
This commit is contained in:
commit
17251cf447
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
Loading…
Reference in New Issue