buildo / eslint-config

buildo's eslint shared config
http://buildo.io
4 stars 2 forks source link

rule proposal: promise plugin #79

Open gabro opened 8 years ago

gabro commented 8 years ago

Essentially this: https://github.com/xjamundx/eslint-plugin-promise

Thoughts? @buildo/frontend

FrancescoCioria commented 8 years ago

Interesting! I don't have (yet?) an opinion about each rule/case, but in general I have the feeling that some of them would be useful immediately.

If more people find this promising I think we should test each rule and report here the current amount of "errors" we have in QIA/AlinIQ

gabro commented 8 years ago

👍 ok for me, wanna list the ones you would include?

ascariandrea commented 8 years ago

As most of the time those rules are already shared, but not enforced, so my personal list would be

Simple and efficient

"promise/always-return": 2

Never saw in our code, anyway enforce it could be helpful

"promise/no-return-wrap": 2,

Not really useful (should we check param names for all lambdas we have)?

"promise/param-names": 0,

I ofter personally omit the catch part of a promise cause I'm confident errors will never happen 🙈

"promise/catch-or-return": 2

"In an ES5 environment, ..." we (buildo) are never in an "ES5 environment" nowadays

"promise/no-native": 0,
giogonzo commented 8 years ago

Not really useful (should we check param names for all lambdas we have)?

no but this one is a very special one ( a "revealing constructor" one https://blog.domenic.me/the-revealing-constructor-pattern/ , lol btw :P )

so I'm in favor of keeping the resolve and reject names consistent

gabro commented 8 years ago

What @ascariandrea proposes is all 👍 for me, with the exception of param-names for which I agree with @giogonzo

giogonzo commented 8 years ago

(keeping param-names I think we end up in the default setting for this rule)

ascariandrea commented 8 years ago

Do we agree in adding this with default settings?

gabro commented 8 years ago

Full force ahead! ✌️ But please make sure to fix projects before merging