Skip to content

Commit cd76ecf

Browse files
bensheldonbensheldon
authored
Merge branch 'main' into remove-upstream-dupes
2 parents ce23317 + 6286185 commit cd76ecf

File tree

6 files changed

+184
-31
lines changed

6 files changed

+184
-31
lines changed

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# RuboCop GitHub ![CI](https://github.com/github/rubocop-github/workflows/CI/badge.svg?event=push)
22

3-
This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects.
3+
This repository provides recommended RuboCop configuration and additional Cops for use on GitHub open source and internal Ruby projects, and is the home of [GitHub's Ruby Style Guide](./STYLEGUIDE.md).
44

55
## Usage
66

‎STYLEGUIDE.md

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ This is GitHub's Ruby Style Guide, inspired by [RuboCop's guide][rubocop-guide].
2727
1. [Conditional keywords](#conditional-keywords)
2828
2. [Ternary operator](#ternary-operator)
2929
17. [Syntax](#syntax)
30+
18. [Rails](#rails)
31+
1. [content_for](#content_for)
32+
2. [Instance Variables in Views](#instance-variables-in-views)
3033

3134
## Layout
3235

@@ -893,4 +896,129 @@ result = hash.map { |_, v| v + 1 }
893896

894897
Refactoring is even better. It's worth looking hard at any code that explicitly checks types.
895898

899+
## Rails
900+
901+
### content_for
902+
903+
Limit usage of `content_for` helper. The use of `content_for` is the same as setting an instance variable plus `capture`.
904+
905+
``` erb
906+
<% content_for :foo do %>
907+
Hello
908+
<% end %>
909+
```
910+
911+
Is effectively the same as
912+
913+
``` erb
914+
<% @foo_content = capture do %>
915+
Hello
916+
<% end %>
917+
```
918+
919+
See "Instance Variables in Views" below.
920+
921+
#### Common Anti-patterns
922+
923+
**Using `content_for` within the same template to capture data.**
924+
925+
Instead, just use `capture`.
926+
927+
``` erb
928+
<!-- bad -->
929+
<% content_for :page do %>
930+
Hello
931+
<% end %>
932+
<% if foo? %>
933+
<div class="container">
934+
<%= yield :page %>
935+
</div>
936+
<% else %>
937+
<%= yield :page %>
938+
<% end %>
939+
```
940+
941+
Simply capture and use a local variable since the result is only needed in this template.
942+
943+
``` erb
944+
<!-- good -->
945+
<% page = capture do %>
946+
Hello
947+
<% end %>
948+
<% if foo? %>
949+
<div class="container">
950+
<%= page %>
951+
</div>
952+
<% else %>
953+
<%= page %>
954+
<% end %>
955+
```
956+
957+
**Using `content_for` to pass content to a subtemplate.**
958+
959+
Instead, `render layout:` with a block.
960+
961+
``` erb
962+
<!-- bad -->
963+
<% content_for :page do %>
964+
Hello
965+
<% end %>
966+
<%= render partial: "page" %>
967+
<!-- _page.html.erb -->
968+
<div class="container">
969+
<%= yield :page %>
970+
</div>
971+
```
972+
973+
Pass the content in a block directly to the `render` function.
974+
975+
``` erb
976+
<!-- good -->
977+
<%= render layout: "page" do %>
978+
Hello
979+
<% end %>
980+
<!-- _page.html.erb -->
981+
<div class="container">
982+
<%= yield %>
983+
</div>
984+
```
985+
986+
### Instance Variables in Views
987+
988+
In general, passing data between templates with instance variables is discouraged. This even applies from controllers to templates, not just between partials.
989+
990+
`:locals` can be used to pass data from a controller just like partials.
991+
992+
``` ruby
993+
def show
994+
render "blob/show", locals: {
995+
:repository => current_repository,
996+
:commit => current_commit,
997+
:blob => current_blob
998+
}
999+
end
1000+
```
1001+
1002+
Rails implicit renders are also discouraged.
1003+
1004+
Always explicitly render templates with a full directory path. This makes template callers easier to trace. You can find all the callers of `"app/view/site/hompage.html.erb"` with a simple project search for `"site/homepage"`.
1005+
1006+
``` ruby
1007+
def homepage
1008+
render "site/homepage"
1009+
end
1010+
```
1011+
1012+
#### Exceptions
1013+
1014+
There are some known edge cases where you might be forced to use instance variables. In these cases, its okay to do so.
1015+
1016+
##### Legacy templates
1017+
1018+
If you need to call a subview that expects an instance variable be set. If possible consider refactoring the subview to accept a local instead.
1019+
1020+
##### Layouts
1021+
1022+
Unfortunately the only way to get data into a layout template is with instance variables. You can't explicitly pass locals to them.
1023+
8961024
[rubocop-guide]: https://github.com/rubocop-hq/ruby-style-guide

‎config/default.yml

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,10 @@ Layout/BlockAlignment:
6565
Layout/BlockEndNewline:
6666
Enabled: true
6767

68+
# TODO: Enable this since it's in the styleguide.
6869
Layout/CaseIndentation:
6970
Enabled: false
71+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#indent-when-as-start-of-case
7072

7173
Layout/ClassStructure:
7274
Enabled: false
@@ -180,16 +182,16 @@ Layout/HeredocIndentation:
180182
Enabled: false
181183

182184
Layout/IndentationConsistency:
183-
Enabled: false
185+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#indentation
184186

185187
Layout/IndentationStyle:
186-
Enabled: true
187188
EnforcedStyle: spaces
188189
IndentationWidth: 2
190+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#default-indentation
189191

190192
Layout/IndentationWidth:
191-
Enabled: true
192193
Width: 2
194+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#default-indentation
193195

194196
Layout/InitialIndentation:
195197
Enabled: true
@@ -258,10 +260,10 @@ Layout/SingleLineBlockChain:
258260
Enabled: false
259261

260262
Layout/SpaceAfterColon:
261-
Enabled: true
263+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators
262264

263265
Layout/SpaceAfterComma:
264-
Enabled: true
266+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators
265267

266268
Layout/SpaceAfterMethodName:
267269
Enabled: true
@@ -270,16 +272,16 @@ Layout/SpaceAfterNot:
270272
Enabled: true
271273

272274
Layout/SpaceAfterSemicolon:
273-
Enabled: true
275+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators
274276

275277
Layout/SpaceAroundBlockParameters:
276278
Enabled: true
277279

278280
Layout/SpaceAroundEqualsInParameterDefault:
279-
Enabled: true
281+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-around-equals
280282

281283
Layout/SpaceAroundKeyword:
282-
Enabled: false
284+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-operators
283285

284286
Layout/SpaceAroundMethodCallOperator:
285287
Enabled: false
@@ -309,8 +311,8 @@ Layout/SpaceInLambdaLiteral:
309311
Enabled: false
310312

311313
Layout/SpaceInsideArrayLiteralBrackets:
312-
Enabled: true
313314
EnforcedStyle: no_space
315+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-spaces-braces
314316

315317
Layout/SpaceInsideArrayPercentLiteral:
316318
Enabled: true
@@ -340,7 +342,7 @@ Layout/TrailingEmptyLines:
340342
Enabled: true
341343

342344
Layout/TrailingWhitespace:
343-
Enabled: true
345+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#trailing-whitespace
344346

345347
Lint/AmbiguousAssignment:
346348
Enabled: false
@@ -678,8 +680,10 @@ Lint/UnreachableCode:
678680
Lint/UnreachableLoop:
679681
Enabled: false
680682

683+
# TODO: Enable this since it's in the styleguide.
681684
Lint/UnusedBlockArgument:
682685
Enabled: false
686+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#underscore-unused-vars
683687

684688
Lint/UnusedMethodArgument:
685689
Enabled: false
@@ -976,8 +980,10 @@ Style/AccessorGrouping:
976980
Style/Alias:
977981
Enabled: false
978982

983+
# TODO: Enable this since it's in the styleguide.
979984
Style/AndOr:
980985
Enabled: false
986+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-and-or-or
981987

982988
Style/ArgumentsForwarding:
983989
Enabled: false
@@ -1076,7 +1082,7 @@ Style/DateTime:
10761082
Enabled: false
10771083

10781084
Style/DefWithParentheses:
1079-
Enabled: true
1085+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#method-parens-when-arguments
10801086

10811087
Style/Dir:
10821088
Enabled: false
@@ -1163,7 +1169,7 @@ Style/FloatDivision:
11631169
Enabled: false
11641170

11651171
Style/For:
1166-
Enabled: true
1172+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#avoid-for
11671173

11681174
Style/FormatString:
11691175
Enabled: false
@@ -1199,8 +1205,8 @@ Style/HashLikeCase:
11991205
Enabled: false
12001206

12011207
Style/HashSyntax:
1202-
Enabled: true
12031208
EnforcedStyle: ruby19_no_mixed_keys
1209+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#symbols-as-keys
12041210

12051211
Style/HashTransformKeys:
12061212
Enabled: false
@@ -1299,7 +1305,7 @@ Style/MultilineIfModifier:
12991305
Enabled: false
13001306

13011307
Style/MultilineIfThen:
1302-
Enabled: true
1308+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-then-for-multi-line-if-unless
13031309

13041310
Style/MultilineInPatternThen:
13051311
Enabled: false
@@ -1475,8 +1481,10 @@ Style/RedundantRegexpCharacterClass:
14751481
Style/RedundantRegexpEscape:
14761482
Enabled: false
14771483

1484+
# TODO: Enable this since it's in the styleguide.
14781485
Style/RedundantReturn:
14791486
Enabled: false
1487+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#avoid-return
14801488

14811489
Style/RedundantSelf:
14821490
Enabled: false
@@ -1563,8 +1571,8 @@ Style/StringHashKeys:
15631571
Enabled: false
15641572

15651573
Style/StringLiterals:
1566-
Enabled: true
15671574
EnforcedStyle: double_quotes
1575+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#double-quotes
15681576

15691577
Style/StringLiteralsInInterpolation:
15701578
Enabled: false
@@ -1626,8 +1634,10 @@ Style/TrailingUnderscoreVariable:
16261634
Style/TrivialAccessors:
16271635
Enabled: false
16281636

1637+
# TODO: Enable this since it's in the styleguide.
16291638
Style/UnlessElse:
16301639
Enabled: false
1640+
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#no-else-with-unless
16311641

16321642
Style/UnlessLogicalOperators:
16331643
Enabled: false

‎config/rails.yml

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,27 @@ GitHub/RailsViewRenderShorthand:
2828

2929
Layout/BlockAlignment:
3030
Exclude:
31-
- app/views/**/*.erb
31+
- "**/*.erb"
3232

3333
Layout/IndentationWidth:
3434
Exclude:
35-
- app/views/**/*.erb
35+
- "**/*.erb"
3636

3737
Layout/InitialIndentation:
3838
Exclude:
39-
- app/views/**/*.erb
39+
- "**/*.erb"
4040

4141
Layout/SpaceInsideParens:
4242
Exclude:
43-
- app/views/**/*.erb
43+
- "**/*.erb"
4444

4545
Layout/TrailingEmptyLines:
4646
Exclude:
47-
- app/views/**/*.erb
47+
- "**/*.erb"
4848

4949
Layout/TrailingWhitespace:
5050
Exclude:
51-
- app/views/**/*.erb
51+
- "**/*.erb"
5252

5353
Lint/UselessAccessModifier:
5454
ContextCreatingMethods:
@@ -393,16 +393,16 @@ Rails/WhereNot:
393393

394394
Style/For:
395395
Exclude:
396-
- app/views/**/*.erb
396+
- "**/*.erb"
397397

398398
Style/OneLineConditional:
399399
Exclude:
400-
- app/views/**/*.erb
400+
- "**/*.erb"
401401

402402
Style/Semicolon:
403403
Exclude:
404-
- app/views/**/*.erb
404+
- "**/*.erb"
405405

406406
Style/StringLiterals:
407407
Exclude:
408-
- app/views/**/*.erb
408+
- "**/*.erb"

0 commit comments

Comments
 (0)