-
Notifications
You must be signed in to change notification settings - Fork 42
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
style(templates:coffee): cleanup & consistency fix of .coffee files #3
Conversation
Re-committed with missing spaces. |
@@ -1,4 +1,5 @@ | |||
'use strict' | |||
|
|||
angular.module('<%= scriptAppName %>').service '<%= classedName %>', <%= classedName %> = -> | |||
angular.module '<%= scriptAppName %>' | |||
.service '<%= classedName %>', <%= classedName %> = -> |
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.
@DaftMonk Any particular reason why we need to "name" function in a second parameter?
This regards to JS
version as well:
angular.module('<%= scriptAppName %>')
.service('<%= classedName %>', function <%= IS_THIS_NAME_NEEDED? %>() {
// AngularJS will instantiate a singleton by calling "new" on this function
});
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, actually that second name isn't really necessary.
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.
Also we should change that to use cameledName name rather than classedName.
style(templates:coffee): cleanup & consistency fix of .coffee files
Oh bummer, just pushed new version with removed unnecessary function naming and fixed to |
I think that will have to be in a separate PR, :-/ sorry about that. |
No prob :). I'll push later today. |
Style fixes to match better with this
Changes include: