search

meanjs / generator-meanjs

MEAN.JS Official Yeoman Generator
http://meanjs.org/
473 stars 177 forks source link

Remove eslint error on crud-module generated files #219

Closed fereloper closed 8 years ago

fereloper commented 8 years ago

Hi There, I created a crud-module with generator-meanjs and run docker run, But it showing error and exited. It took 2 hours to find the error. Finally I saw its about eslint task. It will be good if you see the changes and merge if you think its necessary.

ilanbiala commented 8 years ago

LGTM. @codydaig LGTY?

codydaig commented 8 years ago

According to John Papa: https://github.com/johnpapa/angular-styleguide/blob/master/a1/README.md#iife

However, there are two ways of going about it. https://en.wikipedia.org/wiki/Immediately-invoked_function_expression

Not sure which way we want to be consistent.

demus commented 8 years ago

Personally I think you should stick close to John Papa's guide for consistency, but the entire main repository would have to be refactored as well

codydaig commented 8 years ago

Unless I'm seeing things funky, the code changed in this PR currently uses John Papa's style guide and this PR changes it to the other way.

demus commented 8 years ago

I think you might be seeing things funky.

John Papa's guide uses the wrap-iife inside option: })(); and this PR changes the generator to be consistent with the wrap-iife outside option: }()); used by the main meanjs repository (which is specified in the .eslintrc, hence causing errors when it was John Papa's way). It seems that the eslint default is "outside" anyways: http://eslint.org/docs/rules/wrap-iife

codydaig commented 8 years ago

Ah. I get what your saying now.

@ilanbiala LGTM

Valonix commented 8 years ago

So, will you merge PR?