Orbit-db: Linting

[thiagodelgado111]

Would you folks be interested in adding linting to orbit and its packages? I was thinking something using eslint with an agreed upon standard – I'd recommend using airbnb base style without semicolons. Thoughts?

[haadcode]

I like the idea! 👍

In the past, we didn't feel there was a need for linting as the code and contributor base was small enough to not care. A lot has changed over time and we now have many more people contributing and the code base keeps growing over multiple modules. Now would be a good time to introduce linting.

I agree eslint would be a good tool to use and I like the airbnb rule set as it takes into consideration modern JS and provides sane defaults (in most cases). There are couple of specific we should think about though:

• Semicolons - agreed with @thiagodelgado111 that we shouldn't use semicolons
• import/export are, as far as I know, not supported by Node.js at the moment. We're probably better off to continue using require (for now, review if/when Node.js adds supports)
• Accessors - I feel it makes a lot of sense to use getters/setter. Having worked with Java back in the days, reading the style guide's recommendation of using getVal() and setVal(val) makes me cringe 😄
• We should also make sure that all the modern features/syntax are available in Node.js 8.x. iirc, some of the spread or deconstruction syntax is not yet available, but I may misremember here.
• We should also be willing to review the selected rule set from time to time if there's a need for a change

Question: what would be the best way for all the orbit-db-* modules to share the same lint config?

So yeah, in general I feel this is something we should do, but before adding it, we should make sure the rule set works for us. I'd like to hear more thoughts on this before moving forward!

[thiagodelgado111]

Question: what would be the best way for all the orbit-db-* modules to share the same lint config?

An eslint-config-orbitdb module I guess

[thiagodelgado111]

Semicolons - agreed with @thiagodelgado111 that we shouldn't use semicolons

Actually, I suggested not using semicolons because orbit's codebase doesn't use them and I don't feel strong against that but I actually like semicolons shrug

import/export are, as far as I know, not supported by Node.js at the moment. We're probably better off to continue using require (for now, review if/when Node.js adds supports)

Yes, otherwise we'd have to use babel. How do you feel about that?

Accessors - I feel it makes a lot of sense to use getters/setter. Having worked with Java back in the days, reading the style guide's recommendation of using getVal() and setVal(val) makes me cringe 😄

I don't like the getX/setX style as well :)

We should also make sure that all the modern features/syntax are available in Node.js 8.x. iirc, some of the spread or deconstruction syntax is not yet available, but I may misremember here.

If we use babel (babel-preset-env) we'd be good. It depends on how you feel about that :smile:

We should also be willing to review the selected rule set from time to time if there's a need for a change

Totally agree! I think contributors could just review them from time to time and file issues to change/fix it :)

[thiagodelgado111]

Started writing up a custom config for orbit here: https://github.com/thiagodelgado111/eslint-config-orbitdb

[haadcode]

StandardJS was proposed in the community call some time ago. To move on with this, how about we just use standardjs?

@shamb0t @thiagodelgado111 @RichardLitt thoughts?

[RichardLitt]

I think this is the best move. Standard is relatively easy to use, and the amount of users I've seen tells me that the exceptions to it are mostly trivial. If we need an exception for a particular edge case, I'd be happy to reconsider; but the large suite of tools it has which allow fast onboarding for new contributors makes me think it's the best option for us.

I'm biased; I hate painting bike sheds.

[thiagodelgado111]

I'm up for it as well. We can use eslint-standard config first and if it doesn't fit orbit needs, I can extend it on eslint-orbitdb-config and just add the rules that differ from it, so we don't have to copy everything

[aphelionz]

Linting has been added almost universally :+1: