-
Notifications
You must be signed in to change notification settings - Fork 1
Add Javascript #3
base: master
Are you sure you want to change the base?
Conversation
60c1eb9
to
63b5fc3
Compare
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.
LGTM 😄
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.
LGTM 🙌 GJ @luissifu.
Just some small comments.
|
||
### Useful Links | ||
Styleguide | ||
* [Javascript styleguide](styleguide.md) |
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.
Esto a donde tiene que llevar? Me lleva a un 404 not found (https://github.com/ecaresoft/guides/blob/60c1eb9ad40433549cb9c9e2779478b6327c4c09/javascript/javascript/styleguide.md)
const membershipGoldStub = Service.extend({ | ||
hasLite: false | ||
}); | ||
|
||
test('render lock when template is locked for basic user', function(assert) { | ||
this.container.registry.register('service:mockMembership', membershipBasicStub); | ||
// ... | ||
}); |
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.
Mocking wrong stub membershipGoldStub
-> membershipBasicStub
please change to basic or gold stub 👍
Source: [Apollo](https://github.com/ecaresoft/apollo/blob/master/app/pods/login/controller.js) | ||
|
||
While classes are similar to the other languages, most it's important to understand what the | ||
keywords `import`, `export, and `default` do. |
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.
Missing ` after export
|
||
## Semicolons | ||
|
||
+ Use semicolons`;` |
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.
Add => ... Always
? 🤓
|
||
## Comments | ||
|
||
+ Pad comments with a space. |
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.
Maybe explain when
to use comments as well?
// good | ||
const isTrue = true; | ||
const bar = 123; | ||
let foo; |
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.
Maybe add newline between const
and let
group?
// good | ||
foo[propertyName]; | ||
|
||
// do you even javascript |
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.
🤣 👍
Adds documentation for javascript, including a styleguide and a introduction.
Review using: https://github.com/ecaresoft/guides/tree/60c1eb9ad40433549cb9c9e2779478b6327c4c09