-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Customize for portal #1
base: original-pm-style-guide
Are you sure you want to change the base?
Conversation
README.md
Outdated
- With one key, value pair use a single line | ||
|
||
```coffeescript | ||
car = make: "Toyota" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use double quotes here instead of single? I'd vote for uniform single across the board, except with string interpolation
coffeelint.json
Outdated
"braces_spacing": { | ||
"spaces": 1, | ||
"empty_object_spaces": 1 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what its worth, the coffeelint.json
file in in my PR has all the most updated options for Coffeelint. Might be good to merge those together
Coffeelint.json proposed changes
README.md
Outdated
return @ # No | ||
``` | ||
|
||
Avoid `return` where not required, unless the explicit return increases clarity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might consider also adding something here about explicitly returning long or multi-line chunks of code to improve clarity
<a name="inline_comments"></a> | ||
### Inline Comments | ||
|
||
Inline comments are placed on the line immediately above the statement that they are describing. If the inline comment is sufficiently short, it can be placed on the same line as the statement (separated by a single space from the end of the statement). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should allow comments on the same line. They're more likely to be overlooked and too short to provide any valuable information.
README.md
Outdated
<a name="conditionals"></a> | ||
## Conditionals | ||
|
||
Favor `if` over `unless` for negative conditions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there ever really any circumstances that we want to use unless
? I kind of feel like we should forbid the use of unless
, since an if
statement is always possible.
README.md
Outdated
processPayload() | ||
``` | ||
|
||
If multiple lines are required by the description, indent subsequent lines with two spaces: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this decreases readability. It's just my preference to not indent in this case. If anyone has strong feelings on it, then I'm ok with it, but it just feels awkward to me to indent here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah that sounds fine. We actually decided to use ###
notation for block comments. Although I don't know what counts as a block. > 1 line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, > 1 line seems like a block to me.
README.md
Outdated
`or=` should NOT be used: | ||
|
||
```coffeescript | ||
temp = temp || {} # Yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why wouldn't this line just be temp = temp or {}
?
README.md
Outdated
```coffeescript | ||
# Yes | ||
x(-> 3) | ||
x( -> 3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should maybe be under #No
, because the extra space is unnecessary and makes the arrow function feel unbalanced here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow, this is still on my 'review requests' board.
This is a WIP implementation of Polarmobile's CoffeeScript style guide updated to more closely resemble Portal's current CoffeeScript style. The linter rules should ultimately match the style guide