Skip to content
This repository has been archived by the owner on Jun 17, 2019. It is now read-only.

rule proposal: promise plugin #79

Open
gabro opened this issue Oct 19, 2016 · 8 comments
Open

rule proposal: promise plugin #79

gabro opened this issue Oct 19, 2016 · 8 comments
Labels

Comments

@gabro
Copy link
Member

gabro commented Oct 19, 2016

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

Thoughts? @buildo/frontend

@FrancescoCioria
Copy link
Contributor

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
Copy link
Member Author

gabro commented Oct 19, 2016

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

@ascariandrea
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member Author

gabro commented Oct 19, 2016

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

@giogonzo
Copy link
Member

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

@ascariandrea
Copy link
Contributor

Do we agree in adding this with default settings?

@gabro
Copy link
Member Author

gabro commented Oct 28, 2016

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants